handle errors in getLambdaConfiguration. refactor #18
f/anu-get-code
master f/anu-get-code

@andreineculau

[45]andreineculau opened this pull request

about 1 year ago
Labels

[46]bug

No description given.
andreineculau added the [47]bug label [48]about 1 year ago
andreineculau self-assigned this [49]about 1 year ago
andreineculau requested a review from IanSavchenko [50]about 1 year ago
[51]andreineculau added some commits [52]about 1 year ago

[53]@andreineculau [54]better error message

[55]@andreineculau [56]handle errors in getLambdaConfiguration.

refactor
andreineculau force pushed changes to this branch [57]about 1 year ago
@IanSavchenko

[58]IanSavchenko approved these changes [59]about 1 year ago
[60]andreineculau added a commit [61]about 1 year ago

[62]@andreineculau [63]1.4.5
andreineculau referenced this pull request from commit [64]a9e74e0

[65]about 1 year ago
andreineculau merged commit a9e74e0 into master [66]about 1 year ago
andreineculau deleted the f/anu-get-code branch [67]about 1 month ago

correct versioning. expires 2019-12-31 #34
Closed

[57]andreineculau opened this issue Aug 14, 2019 · 1 comment

Closed
[58]correct versioning. expires 2019-12-31 #34
[59]andreineculau opened this issue Aug 14, 2019 · 1 comment

Assignees

[60]@andreineculau

Labels

[61]bug
Comments
[62]@andreineculau

Copy link (BUTTON) Quote reply

Member
[63]@andreineculau [64]andreineculau commented [65]Aug 14, 2019 •
edited
 * [X] v2.1.1 -> v0.2.11
 * [X] internal repos depending on aws-util-firecloud should switch
   asap
 * [X] remove v[^0].+ tags by 2019-12-31

[66]@andreineculau [67]andreineculau added the [68]bug label [69]Aug

14, 2019

[70]@andreineculau [71]andreineculau self-assigned this [72]Aug 14,

2019

[73]@andreineculau [74]andreineculau added this to To do in [75]Public

(Open Source) via automation [76]Aug 14, 2019

[77]andreineculau added a commit that referenced this issue [78]Aug 14,

2019

[79]@andreineculau

[80]disclaimer for upcoming commit to correct versioning v2.1.1 ->

v0.2.1… (BUTTON) …

Verified

This commit was signed with a verified signature.

[81]andreineculau Andrei Neculau

GPG key ID: 79FA7EE650BF9A61 [82]Learn about signing commits

[83]695ff07

…1. see [84]#34
[85]@andreineculau [86]andreineculau changed the title [DEL: correct

versioning :DEL] [INS: correct versioning. expires 2019-12-31 :INS]

[87]Aug 16, 2019

[88]@andreineculau
This comment has been minimized.
[89]Sign in to view

Copy link (BUTTON) Quote reply

Member Author
[90]@andreineculau [91]andreineculau commented [92]Feb 13, 2020
v[^0].+ tags have now been deleted

[93]@andreineculau [94]andreineculau closed this [95]Feb 13, 2020

[96]Public (Open Source) automation moved this from To do to Done

[97]Feb 13, 2020

[98]@ankitmth [99]ankitmth removed this from Done in [100]Public (Open

Source) [101]Feb 19, 2020

refactor express to handle ResponseErrors #15
f/anu-api-bootstrap
master f/anu-api-bootstrap

@andreineculau

[45]andreineculau opened this pull request about 1 year ago • edited

about 1 year ago
Labels

[46]enhancement
long road to basically remove the need for api-handler-bootstrap.js

from atex-platform

so the core of this review is in express/res-error.js, otherwise it's

just breaking up express.js
[47]andreineculau added a commit [48]about 1 year ago

[49]@andreineculau [50]refactor express to handle ResponseErrors
andreineculau added the [51]enhancement label [52]about 1 year ago
andreineculau self-assigned this [53]about 1 year ago
andreineculau requested a review from IanSavchenko [54]about 1 year ago
@IanSavchenko

[55]IanSavchenko approved these changes [56]about 1 year ago
The code needs restructuring, but maybe next time.

(BUTTON) Show resolved (BUTTON) Hide resolved

[57]src/express/middlewares.js

((6 lines not shown))

asyncHandler,

getRequestInstance

} from '../lambda';

import {

bootstrap as bootstrapResponseError

} from './res-error';

export let bootstrap = function(fn) {

return asyncHandler(async function(req, res, next) {

bootstrapResponseError(await _.alwaysPromise(fn(req, res, next)), res);

});

};

export let init = function(req, res, next) {

res.instance = getRequestInstance(req);

[58]@IanSavchenko [59]IanSavchenko • [60]about 1 year ago

Copy link
I understand that this is not part of this PR, but this line is

misleading if you are out of context. We assign a property instance for

the response which is an instance of request according to the

right-hand value 🤔

But in fact it's much easier: that's just a request id from AWS. So why

not calling things what they are?

[61]@andreineculau [62]andreineculau • [63]about 1 year ago

Copy link
some history was erased when publishing this repo, but

getRequestInstance will return

${ctx.invokedFunctionArn}#request:${ctx.awsRequestId}, so not just the

request id. Basically it makes it easier to check lambda's logs in

cloudwatch.
I made a subtle change that maybe makes it less misleading, because the

code in question is more contextualized now

(BUTTON) Show resolved (BUTTON) Hide resolved

[64]src/express/res-mixins.js

((17 lines not shown))

this.setHeader('link', linkHeader);

};

export let send = function(body, mediaType) {

this.send = this.oldSend;

if (mediaType) {

this.set('content-type', mediaType);

}

if (!_.isUndefined(this.validate) &&

_.startsWith(this.get('content-type'), this.validate.schema.mediaType)

) {

let valid = this.validate(body);

if (!valid) {

this.log.error({

[65]@IanSavchenko [66]IanSavchenko • [67]about 1 year ago

Copy link
I think it's misleading when we spit error but continue working. I

would change this to warn.
Copy link

@andreineculau

[68]andreineculau

commented [69]about 1 year ago
 The code needs restructuring,

Forgot to ask what did you mean
Copy link

@IanSavchenko

[70]IanSavchenko

commented [71]about 1 year ago
 The code needs restructuring,

 Forgot to ask what did you mean

I can explain. For example, middlewares.js could be broken down further

to 3 separate parts:

1. Method middlewares.bootstrap is actually our wrapper for all

middlewares which is used in our redefined app.use. So the code can

be moved out there since it's the only place where we use it. So it

will be like
app.oldUse = app.use;

app.use = function(fn) {

// not the best name surely

let wrappedFn = asyncHandler(async function(req, res, next) {

bootstrapResponseError(await _.alwaysPromise(fn(req, res, next)), res);

});

app.oldUse(wrappedFn);

};
2. Method middlewares.init is used like a middleware, but it's unclear
   from it's name what is the purpose. Why not call it
   mixins-middleware and then put it aside next to modules
   mixins-req.js and mixins-res.js. Would be nice.
3. Method middlewares.xForward is just a plain middleware by itself,
   not a service-wrapper-function (like bootstrap), not a complex
   meta-middleware with some extra modules (like init/mixins).

So, for me all three of these methods are serving completely different

purposes and should be split into separate files.
Also, I would move out bootstrap from res-error.js, but that's not

super important.

👍 1

andreineculau reacted with thumbs up emoji
Copy link

@andreineculau

[72]andreineculau

commented [73]about 1 year ago

1. 👏 pushed a change (and fix along with it)

2. renamed init to applyMixins
[74]andreineculau added some commits [75]about 1 year ago

[76]@andreineculau [77]warn instead of error

[78]@andreineculau [79]get request instance only in case of sending

errors

[80]@andreineculau [81]refactor bootstrapMiddleware

[82]@andreineculau [83]rename init middleware to applyMixins

[84]@andreineculau [85]break up lambda module, replace bunyan with

minlog

[86]@andreineculau [87]fixup! break up lambda module, replace bunyan

with minlog

[88]@andreineculau [89]use minlog's levelIsBeyondGroup convenience

function

[90]@andreineculau [91]Merge pull request [92]#16 [93]from

tobiipro/f/anu-minlog
andreineculau referenced this pull request from commit [94]2b60c4e

[95]about 1 year ago
andreineculau merged commit 2b60c4e into master [96]about 1 year ago
andreineculau deleted the f/anu-api-bootstrap branch [97]about 1 year

ago

try removing gc on demand #19
Closed

[57]andreineculau opened this issue Feb 5, 2019 · 1 comment

Closed
[58]try removing gc on demand #19
[59]andreineculau opened this issue Feb 5, 2019 · 1 comment

Labels

[60]enhancement
Comments
[61]@andreineculau

Copy link (BUTTON) Quote reply

Member
[62]@andreineculau [63]andreineculau commented [64]Feb 5, 2019
[65]https://github.com/tobiipro/aws-util-firecloud/blob/master/src/lamb

da.js#L59-L66
I believe node 8 has some gc changes, so maybe, just maybe we don't

need to spend time running garbage collection on demand.
I suggest we try removing it, and then see the behaviour. We could also

add a flag, but I don't particularly see the point in it.
// cc [66]@tobiiasl

[67]@andreineculau [68]andreineculau added the [69]enhancement label

[70]Feb 5, 2019

[71]@andreineculau
This comment has been minimized.
[72]Sign in to view

Copy link (BUTTON) Quote reply

Member Author
[73]@andreineculau [74]andreineculau commented [75]Mar 14, 2019
Leave as is for now

[76]@andreineculau [77]andreineculau closed this [78]Mar 14, 2019

Typescript #35
typescript
master typescript

@andreineculau

[45]andreineculau opened this pull request 4 months ago • edited 4

months ago
Labels

[46]enhancement
Linked issues

[47]add typescript definition

* Fixes: [48]#31

* Breaking change: [x]

__________________________________________________________________
[49]andreineculau added some commits [50]4 months ago

[51]@andreineculau [52]rename .js to .ts

[53]@andreineculau [54]enable typescript

[55]@andreineculau [56]typescript

[57]@andreineculau [58]please typescript

[59]@andreineculau [60]typescript: package.json is not under rootDir

[61]@andreineculau [62]lint

[63]@andreineculau [64]updated support-firecloud
andreineculau added the [65]enhancement label [66]4 months ago
andreineculau self-assigned this [67]4 months ago
andreineculau added this to In progress in [68]Public (Open Source) via

automation [69]4 months ago
[70]andreineculau added some commits [71]3 months ago

[72]@andreineculau [73]sync with minlog-flush

[74]@andreineculau [75]breaking: remove export default exports

[76]@andreineculau [77]normalize flushing the lambda logger

[78]@andreineculau [79]style: less chaining

[80]@andreineculau [81]fix imports without defaults

[82]@andreineculau [83]await for spy2called

[84]@andreineculau [85]await for spy2called

[86]@andreineculau [87]fix import default

[88]@andreineculau [89]make sure we don't log to console after test

case

[90]@andreineculau [91]use wait-for-expect

[92]@andreineculau [93]remove old eslint plugins

[94]@andreineculau [95]updated support-firecloud

[96]@andreineculau [97]fix tests (extra careful with spying on

console), deduplicate

[98]@andreineculau [99]fix for eslint-config-firecloud

[100]@andreineculau [101]lint
andreineculau merged commit 4396259 into master [102]about 2 months ago
Public (Open Source) automation moved this from In progress to Done

[103]about 2 months ago
andreineculau deleted the typescript branch [104]about 2 months ago
ankitmth removed this from Done in [105]Public (Open Source) [106]17

days ago

add sqs utils #28
f/anu-sqs
master f/anu-sqs

@andreineculau

[45]andreineculau opened this pull request

10 months ago
Labels

[46]enhancement

* Fixes:

* Breaking change: [ ]

__________________________________________________________________
andreineculau added the [47]enhancement label [48]10 months ago
andreineculau requested a review from IanSavchenko [49]10 months ago
andreineculau self-assigned this [50]10 months ago
@IanSavchenko

[51]IanSavchenko reviewed [52]10 months ago
Nice! I will add more stuff after you merge this.

(BUTTON) Show resolved (BUTTON) Hide resolved [53]src/sqs.js

@@ -0,0 +1,18 @@

import _ from 'lodash-firecloud';

import aws from 'aws-sdk';

export let getNameFromArn = _.memoize(function(queueArn) {

let name = /arn:aws:sqs:.:.:(.*)/.exec(queueArn)[1];

return name;

});

export let getUrl = _.memoize(async function(queueName) {

[54]@IanSavchenko [55]IanSavchenko • [56]10 months ago

Copy link
I would suggest that we pass sqs instance as a named arg together with

queueName. It can then have a default value of new aws.SQS(), so like

export let getUrl = _.memoize(async function({queueName, sqs = new aws.SQS()}) {

...
[57]@andreineculau [58]andreineculau • [59]10 months ago

Copy link
I actually had the thought but i couldn't come up with a reason for it.

Credentials maybe, now when i think about it.

Will do it, and i guess it is good to do it for all other funs in

aws-util-firecloud

[60]@andreineculau [61]andreineculau • [62]10 months ago

Copy link
I so your comment about discoverability after i pushed this. I guess

that can be addressed properly via typescript defs.
Anything i can do to allure you into writing defs for this repi like

you did for lodash-firecloud? 🤔🤗

[63]@andreineculau [64]andreineculau • [65]10 months ago

Copy link
done, except you cannot memoize now (you can, but it makes too complex.

the caller can take care of it

[66]@IanSavchenko [67]IanSavchenko • [68]10 months ago

Copy link
 Anything i can do to allure you into writing defs for this repi like
 you did for lodash-firecloud? 🤔🤗

I can re-write it to typescript ;)

[69]@IanSavchenko [70]IanSavchenko • [71]10 months ago

Copy link
I have to say that without memoization, it has not so much benefit

anymore...

[72]@andreineculau [73]andreineculau • [74]10 months ago

Copy link

1. memoization is an optimization, simply cannot be core functionality

2. one liner vs 10+, useful in generic places sounds like enough
andreineculau force pushed changes to this branch [75]10 months ago
[76]andreineculau added a commit [77]10 months ago

[78]@andreineculau [79]add sqs utils
andreineculau force pushed changes to this branch [80]10 months ago
andreineculau added this to Review in progress in [81]Public (Open

Source) [82]10 months ago
andreineculau closed this pull request [83]10 months ago
Public (Open Source) automation moved this from Review in progress to

Done [84]10 months ago
andreineculau deleted the f/anu-sqs branch [85]about 1 month ago
ankitmth removed this from Done in [86]Public (Open Source) [87]17 days

ago

aws-sdk logger: handle possible regexp exception #32
fix-logger-regexp
master fix-logger-regexp

@IanSavchenko

[45]IanSavchenko opened this pull request

9 months ago

* Fixes:

* Breaking change: [ ]

__________________________________________________________________
Ian Savchenko added a commit [46]9 months ago

[47]aws-sdk logger: handle possible regexp exception
IanSavchenko requested a review from andreineculau [48]9 months ago
@andreineculau

[49]andreineculau approved these changes [50]9 months ago
andreineculau referenced this pull request from commit [51]1af484d

[52]9 months ago
andreineculau merged commit 1af484d into master [53]9 months ago
IanSavchenko deleted the fix-logger-regexp branch [54]9 months ago
andreineculau referenced this pull request from commit [55]0853afc

[56]7 months ago

Didn't see Slack notification from this repos Travis on new tag pushed #3
Closed

[57]IanSavchenko opened this issue Mar 1, 2018 · 1 comment

Closed
[58]Didn't see Slack notification from this repos Travis on new tag pushed #3
[59]IanSavchenko opened this issue Mar 1, 2018 · 1 comment
Comments
[60]@IanSavchenko

Copy link (BUTTON) Quote reply

Contributor
[61]@IanSavchenko [62]IanSavchenko commented [63]Mar 1, 2018
No description provided.

[64]@IanSavchenko
This comment has been minimized.
[65]Sign in to view

Copy link (BUTTON) Quote reply

Contributor Author
[66]@IanSavchenko [67]IanSavchenko commented [68]Mar 9, 2018
Seems like we fixed it. It was a regex issue.

[69]@IanSavchenko [70]IanSavchenko closed this [71]Mar 9, 2018

remove cors() from lambda express, maybe reexport cors module #27
Closed

[57]IanSavchenko opened this issue Mar 29, 2019 · 1 comment

Closed
[58]remove cors() from lambda express, maybe reexport cors module #27
[59]IanSavchenko opened this issue Mar 29, 2019 · 1 comment

Labels

[60]enhancement
Comments
[61]@IanSavchenko

Copy link (BUTTON) Quote reply

Contributor
[62]@IanSavchenko [63]IanSavchenko commented [64]Mar 29, 2019
We discussed that it should be up to the API lambda itself to setup

CORS, so need to remove it here, mark the change as breaking and update

lambdas.
Regarding reexporting CORS: I wouldn't do it, because clients get to

decide which version to use and type-inference will probably work

better (if we decide to use TS in lambda at some point).

(THUMBS_UP react) 👍 1

[65]@IanSavchenko [66]IanSavchenko added the [67]enhancement label

[68]Mar 29, 2019

[69]@andreineculau [70]andreineculau added this to To do in [71]Public

(Open Source) [72]May 20, 2019

[73]andreineculau added a commit that referenced this issue [74]Aug 16,

2019

[75]@andreineculau

[76]allow for defaultMiddlewares to be replaced. ref [77]#27

Verified

This commit was signed with a verified signature.

[78]andreineculau Andrei Neculau

GPG key ID: 79FA7EE650BF9A61 [79]Learn about signing commits

[80]0a14b2b

[81]andreineculau added a commit that referenced this issue [82]Aug 16,

2019

[83]@andreineculau

[84]add convenience function to disable a default middleware. ref

[85]#27

Verified

This commit was signed with a verified signature.

[86]andreineculau Andrei Neculau

GPG key ID: 79FA7EE650BF9A61 [87]Learn about signing commits

[88]0aadea2

[89]@andreineculau
This comment has been minimized.
[90]Sign in to view

Copy link (BUTTON) Quote reply

Member
[91]@andreineculau [92]andreineculau commented [93]Aug 16, 2019
closing. the expressApp (that is accessible in a express-bootstrapped

lambda handler) now surfaces a property defaultMiddlewares. those

middlewares can now be manipulated

import cors from 'cors';
import {

bootstrap

} from 'aws-util-firecloud/lib/express';
export let handle = bootstrap(async function(app, _e, _ctx) {

app.defaultMiddlewares.cors.disable();

app.use(cors(...));

});
[94]@andreineculau [95]andreineculau closed this [96]Aug 16, 2019

[97]Public (Open Source) automation moved this from To do to Done

[98]Aug 16, 2019

[99]@ankitmth [100]ankitmth removed this from Done in [101]Public (Open

Source) [102]Feb 19, 2020

breaking: simplify cfn build function #10
f/less-cfn-magic
master f/less-cfn-magic

@andreineculau

[45]andreineculau opened this pull request

over 1 year ago
Labels

[46]enhancement
Q: we said we bump the minor version for our modules when we have

backward incompat, right?
andreineculau added the [47]enhancement label [48]over 1 year ago
andreineculau requested a review from IanSavchenko [49]over 1 year ago
Copy link

@IanSavchenko

[50]IanSavchenko

commented [51]over 1 year ago
I have a vague memory of these talks. I thought we said that we are

going to stick to classical semantic versioning because nothing is

stopping us?
@IanSavchenko

[52]IanSavchenko reviewed [53]over 1 year ago
Cool! Maybe you could also split into separate function(s) code under

if (!partial) { while you are here? :)
Copy link

@andreineculau

[54]andreineculau

commented [55]over 1 year ago
re: versions --- you made me check black-on-white now :) I think we

started with saying that (what you said), and then we reverted to what

I said. Reality is like I said though:

"babel-loader": "^7.1.3",

"babel-preset-firecloud": "git://github.com/tobiipro/babel-preset-firecloud.

git#semver:~0.1.4",
re: partial --- 👍
[56]andreineculau added a commit [57]over 1 year ago

[58]@andreineculau [59]breaking: simplify cfn build function
andreineculau force pushed changes to this branch [60]over 1 year ago
[61]andreineculau added a commit [62]over 1 year ago

[63]@andreineculau [64]fix installing as a raw dep
andreineculau force pushed changes to this branch [65]over 1 year ago
@IanSavchenko

[66]IanSavchenko approved these changes [67]over 1 year ago
IanSavchenko referenced this pull request from commit [68]739cf26

[69]over 1 year ago
IanSavchenko merged commit 739cf26 into master [70]over 1 year ago
andreineculau referenced this pull request from commit [71]0cdd910

[72]over 1 year ago
andreineculau deleted the f/less-cfn-magic branch [73]over 1 year ago

simplify src/cfn/lambda.js : add #12
Closed

[57]andreineculau opened this issue Dec 9, 2018 · 1 comment

Closed
[58]simplify src/cfn/lambda.js : add #12
[59]andreineculau opened this issue Dec 9, 2018 · 1 comment

Assignees

[60]@andreineculau

Labels

[61]enhancement
Comments
[62]@andreineculau

Copy link (BUTTON) Quote reply

Member
[63]@andreineculau [64]andreineculau commented [65]Dec 9, 2018
no config should be needed, no assumptions about storage resources etc
basically we need to get it down to "give me the Code and the

Environment.Variables"
PS: breaking change, bump minor

[66]@andreineculau [67]andreineculau added the [68]enhancement label

[69]Dec 9, 2018

[70]@andreineculau [71]andreineculau self-assigned this [72]Dec 9, 2018

[73]@IanSavchenko
This comment has been minimized.
[74]Sign in to view

Copy link (BUTTON) Quote reply

Contributor
[75]@IanSavchenko [76]IanSavchenko commented [77]Dec 9, 2018
Yes!!!!

(LAUGH react) 😄 1

[78]@andreineculau [79]andreineculau added this to To do in [80]Public

(Open Source) [81]May 20, 2019

[82]andreineculau added a commit that referenced this issue [83]Aug 15,

2019

[84]@andreineculau

[85]split lambda, prepping for simplifications. ref [86]#12

Verified

This commit was signed with a verified signature.

[87]andreineculau Andrei Neculau

GPG key ID: 79FA7EE650BF9A61 [88]Learn about signing commits

[89]addb66b

[90]@andreineculau [91]andreineculau closed this in [92]7cf852a [93]Aug

16, 2019

[94]Public (Open Source) automation moved this from To do to Done

[95]Aug 16, 2019

[96]@ankitmth [97]ankitmth removed this from Done in [98]Public (Open

Source) [99]Feb 19, 2020

note: native async code has short stacktraces #23
Closed

[57]andreineculau opened this issue Feb 22, 2019 · 2 comments · Fixed

by [58]tobiipro/babel-preset-firecloud#13

Closed
[59]note: native async code has short stacktraces #23
[60]andreineculau opened this issue Feb 22, 2019 · 2 comments · Fixed

by [61]tobiipro/babel-preset-firecloud#13
Comments
[62]@andreineculau

Copy link (BUTTON) Quote reply

Member
[63]@andreineculau [64]andreineculau commented [65]Feb 22, 2019
apparently proper stack traces only come in node 12, and under a flag

(at least that's the current state)
maybe the babel-config-firecloud should go back to transforming

(generators or bluebird) at least for node :(

[66]@andreineculau
This comment has been minimized.
[67]Sign in to view

Copy link (BUTTON) Quote reply

Member Author
[68]@andreineculau [69]andreineculau commented [70]Feb 22, 2019
or transform code to do smth like

[71]https://github.com/amio-io/await-trace

[72]@andreineculau [73]andreineculau mentioned this issue [74]Feb 25,

2019

[75]add await-trace plugin for async stacktraces #13

Merged

[76]@andreineculau
This comment has been minimized.
[77]Sign in to view

Copy link (BUTTON) Quote reply

Member Author
[78]@andreineculau [79]andreineculau commented [80]May 13, 2019
node 12 has async stack traces without the flag
NOTE: we need to switch aws lambda to nodejs12 layer, if we want to ---

lambdaci layers [81]https://github.com/lambci/node-custom-lambda

[82]@andreineculau [83]andreineculau closed this in

[84]tobiipro/babel-preset-firecloud#13 [85]May 14, 2019

investigate regression #38
Open

[57]andreineculau opened this issue Feb 13, 2020 · 0 comments

Open
[58]investigate regression #38
[59]andreineculau opened this issue Feb 13, 2020 · 0 comments

Labels

[60]bug
Comments
[61]@andreineculau

Copy link (BUTTON) Quote reply

Member
[62]@andreineculau [63]andreineculau commented [64]Feb 13, 2020
related to emergency fix [65]104b634

[66]@andreineculau [67]andreineculau added the [68]bug label [69]Feb

13, 2020

[70]@andreineculau [71]andreineculau added this to To do in [72]Public

(Open Source) via automation [73]Feb 13, 2020

[74]@ankitmth [75]ankitmth removed this from To do in [76]Public (Open

Source) [77]Feb 19, 2020

break up lambda module, replace bunyan with minlog #16
f/anu-minlog
f/anu-api-bootstrap f/anu-minlog

@andreineculau

[45]andreineculau opened this pull request about 1 year ago • edited

about 1 year ago
Labels

[46]enhancement
once again the core change is lambda/logger.js, I broke up lambda.js

* [X] depends on PRs in minlog
andreineculau added the [47]enhancement label [48]about 1 year ago
andreineculau self-assigned this [49]about 1 year ago
andreineculau requested a review from IanSavchenko [50]about 1 year ago
andreineculau force pushed changes to this branch [51]about 1 year ago
@IanSavchenko

[52]IanSavchenko reviewed [53]about 1 year ago
Great break up!

(BUTTON) Show resolved (BUTTON) Hide resolved [54]src/lambda/logger.js

((54 lines not shown))

ctx.log.trace('Making an AWS SDK call.', {

aws: {

serviceIdentifier,

status,

delta,

retryCount,

operation,

params

}

});

}

};

};

let _setupLongStacktraces = function({ctx}) {

if (ctx.log.level() >= ctx.log.levelToLevelCode('TRACE')) {

[55]@IanSavchenko [56]IanSavchenko • [57]about 1 year ago

Copy link
Here and in other places, you expect ctx.log.level() to return a

number, I guess, to compare it with level code for TRACE. But I can

clearly see that the value it will return will be string like INFO (see

setup, line 101).

[58]@andreineculau [59]andreineculau • [60]about 1 year ago

Copy link
❤️

(BUTTON) Show resolved (BUTTON) Hide resolved [61]src/lambda/logger.js

((86 lines not shown))

let level = _.get(ctx, 'env.LOG_LEVEL', 'INFO');

let logger = new MinLog({

serializers: [

serializeTime,

serializeErr,

_makeLambdaSerializer({ctx})

],

listeners: [

logToConsole({

level

})

]

});

logger.level = function() {

[62]@IanSavchenko [63]IanSavchenko • [64]about 1 year ago

Copy link
I can hardly see why this function is needed. We have several places

with logic around if it is TRACE log level or not for some extra

debugging info. Why just not set a flag ctx.trace = true once and read

it everywhere?

[65]@andreineculau [66]andreineculau • [67]about 1 year ago

Copy link
i made a change in the spirit of of what you said, but it's on ctx.log

not ctx
[68]andreineculau added some commits [69]about 1 year ago

[70]@andreineculau [71]break up lambda module, replace bunyan with

minlog

[72]@andreineculau [73]fixup! break up lambda module, replace bunyan

with minlog
andreineculau force pushed changes to this branch [74]about 1 year ago
[75]andreineculau added a commit [76]about 1 year ago

[77]@andreineculau [78]use minlog's levelIsBeyondGroup convenience

function
andreineculau referenced this pull request from commit [79]3f3296f

[80]about 1 year ago
andreineculau merged commit 3f3296f into f/anu-api-bootstrap [81]about

1 year ago
andreineculau deleted the f/anu-minlog branch [82]about 1 year ago
Copy link

@andreineculau

[83]andreineculau commented [84]about 1 year ago • edited about 1 year

ago
fixes [85]#13 [86]#5

Make sure 'region' and 'account' don't fail without env variables defined #8
Closed

[57]IanSavchenko opened this issue Apr 12, 2018 · 0 comments

Closed
[58]Make sure 'region' and 'account' don't fail without env variables defined

#8
[59]IanSavchenko opened this issue Apr 12, 2018 · 0 comments

Assignees

[60]@IanSavchenko
Comments
[61]@IanSavchenko

Copy link (BUTTON) Quote reply

Contributor
[62]@IanSavchenko [63]IanSavchenko commented [64]Apr 12, 2018
No description provided.

(HOORAY react) 🎉 1

[65]@IanSavchenko [66]IanSavchenko self-assigned this [67]Apr 24, 2018

[68]@IanSavchenko [69]IanSavchenko closed this in [70]f26d330 [71]Apr

24, 2018

F/anu api bootstrap 2 #21
f/anu-api-bootstrap-2
master f/anu-api-bootstrap-2

@andreineculau

[45]andreineculau opened this pull request about 1 year ago • edited

about 1 year ago
Labels

[46]bug

* Fixes:

* Breaking change: [x]

__________________________________________________________________
breaking changes:

* removed

+ asyncHandler

+ bootstrap (express-middleware.js)

+ res.sendError [DEL: should maybe even remove res.sendError

mixin? since it's eclipsed by throw new ResponseError() :DEL]

🤔

+ 2nd argument (type) in res.send

* non-http lambdas need to throw or return the response

* http (express) lambdas need to throw (preferably ResponseError) or

call res.(s)end or return the response

* no need to wrap middlewares anymore, just the handlers with the

bootstrap function from aws-util-firecloud/lib/{lambda,express}
andreineculau force pushed changes to this branch [47]about 1 year ago
andreineculau requested a review from IanSavchenko [48]about 1 year ago
andreineculau self-assigned this [49]about 1 year ago
andreineculau added the [50]bug label [51]about 1 year ago
andreineculau modified this pull request [52]about 1 year ago
Copy link

@andreineculau

[53]andreineculau

commented [54]about 1 year ago
[55]@IanSavchenko

changes since yesterday: I'm still monkey-patching the Layer's

handle_error and handle_request but in a simple way - just handle async

handlers across the board. The handlers, still need to call next or

res.(s)end.
andreineculau force pushed changes to this branch [56]about 1 year ago
[57]andreineculau added some commits [58]about 1 year ago

[59]@andreineculau [60]wrap not just app.use, but app.,

router.all, router.,…

[61]@andreineculau [62]use _.callbackify

[63]@andreineculau [64]bump minlog

[65]@andreineculau [66]upgrade minlog

[67]@andreineculau [68]fix middleware signature

[69]@andreineculau [70]remove old log entry

[71]@andreineculau [72]no need for async

[73]@andreineculau [74]change bootstrapping signatures to async

all-the-way

[75]@andreineculau [76]simplify

[77]@andreineculau [78]return response

[79]@andreineculau [80]no res.log anymore

[81]@andreineculau [82]fix await/async flow

[83]@andreineculau [84]regular express middlewares need to be

promisified first

[85]@andreineculau [86]ignore default export

[87]@andreineculau [88]moar fixes around async funs

[89]@andreineculau [90]bump lodash-firecloud

[91]@andreineculau [92]handle cases where res.sendError is not yet

attached

[93]@andreineculau [94]pass next to layer functions

[95]@andreineculau [96]move response error handler to an express error

handler

[97]@andreineculau [98]refactor async/callback glue

[99]@andreineculau [100]express error handlers need to be registered

last

[101]@andreineculau [102]remove double try-catch

[103]@andreineculau [104]using async-await instead of callbackify

[105]@andreineculau [106]if calling was next twice, throw instead of

giving warning

[107]@andreineculau [108]hide res.sendError

[109]@andreineculau [110]trace lambda result

[111]@andreineculau [112]send request id to logger

[113]@andreineculau [114]fix error handling

[115]@andreineculau [116]correct usage of express res.send

[117]@andreineculau [118]clarify message

[119]@andreineculau [120]breaking: remove type from res.send(body,

type)

[121]@andreineculau [122]fix undefined logger

[123]@andreineculau [124]use new minlog's aws-lambda console logging
andreineculau force pushed changes to this branch [125]about 1 year ago
[126]andreineculau added a commit [127]about 1 year ago

[128]@andreineculau [129]enable babelSrc
andreineculau force pushed changes to this branch [130]about 1 year ago
[131]andreineculau added some commits [132]about 1 year ago

[133]@andreineculau [134]Revert "remove double try-catch"

[135]@andreineculau [136]handle async middleware in express Layer

[137]@andreineculau [138]kill the lambda on not-ResponseError throws,

and process.exit

[139]@andreineculau [140]remove check for calling next twice

[141]@andreineculau [142]call express next with err when headersSent,

as per express' docs

[143]@andreineculau [144]clarify naming
Copy link

@andreineculau

[145]andreineculau

commented [146]about 1 year ago
[147]@IanSavchenko ping. pushed the new stuff as well. thanks!
[148]andreineculau added some commits [149]about 1 year ago

[150]@andreineculau [151]add tests

[152]@andreineculau [153]use babel-preset-firecloud release
andreineculau force pushed changes to this branch [154]about 1 year ago
@IanSavchenko

[155]IanSavchenko approved these changes [156]about 1 year ago
[157]andreineculau added some commits [158]about 1 year ago

[159]@andreineculau [160]make express bootstrap pass errors to lambda

bootstrap, which will pr…

[161]@andreineculau [162]remove mockReset calls
@IanSavchenko

[163]IanSavchenko approved these changes [164]about 1 year ago
andreineculau referenced this pull request from commit [165]6830b72

[166]about 1 year ago
andreineculau merged commit 6830b72 into master [167]about 1 year ago
andreineculau deleted the f/anu-api-bootstrap-2 branch [168]about 1

month ago

bump @types/node to ^13.5.1 #37
Closed

[57]andreineculau opened this issue Jan 29, 2020 · 0 comments

Closed
[58]bump @types/node to ^13.5.1 #37
[59]andreineculau opened this issue Jan 29, 2020 · 0 comments
Comments
[60]@andreineculau

Copy link (BUTTON) Quote reply

Member
[61]@andreineculau [62]andreineculau commented [63]Jan 29, 2020
No description provided.

[64]@andreineculau [65]andreineculau added this to To do in [66]Public

(Open Source) via automation [67]Jan 29, 2020

[68]@andreineculau [69]andreineculau closed this in [70]ff7b8bd [71]Jan

29, 2020

[72]Public (Open Source) automation moved this from To do to Done

[73]Jan 29, 2020

[74]@ankitmth [75]ankitmth removed this from Done in [76]Public (Open

Source) [77]Feb 19, 2020

surface the internal aws-cfn-util-firecloud repo #2
f/anu-merge-aws-cfn-util-firecloud
master f/anu-merge-aws-cfn-util-firecloud

@andreineculau

[45]andreineculau opened this pull request

about 2 years ago
Goal

* remove internal/project-specific constants from

aws-cfn-util-firecloud

* consolidate aws-util-firecloud with cfn-specific funs

* make it possible for public repos to include AWS CloudFormation

templates that can be executed
[46]andreineculau referenced this pull request from another pull

request

[47]#2 document cfn bootstraping in tobiipro/support-firecloud
andreineculau force pushed changes to this branch [48]about 2 years ago
andreineculau force pushed changes to this branch [49]about 2 years ago
andreineculau force pushed changes to this branch [50]about 2 years ago
andreineculau force pushed changes to this branch [51]about 2 years ago
[52]andreineculau added some commits [53]about 2 years ago

[54]@andreineculau [55]move bootstrap from aws-cfn-util-firecloud (no

history necessary)

[56]@andreineculau [57]move logger to its own module

[58]@andreineculau [59]move env proxy from aws-cfn-util-firecloud

[60]@andreineculau [61]move config from aws-cfn-util-firecloud

[62]@andreineculau [63]whitespace

[64]@andreineculau [65]always export all=exports

[66]@andreineculau [67]rename getAwsConfig to get. redundant

[68]@andreineculau [69]merge aws-cfn-util-firecloud's non-cfn modules

[70]@andreineculau [71]merge aws-cfn-util-firecloud's cfn modules

[72]@andreineculau [73]put lambda-related funs into authority module

[74]@andreineculau [75]generalize path to base template

[76]@andreineculau [77]base template can simply be added to incs

[78]@andreineculau [79]generic accounts - not just dev and prod
andreineculau force pushed changes to this branch [80]about 2 years ago
andreineculau referenced this pull request from commit [81]2bc49b4

[82]about 2 years ago
andreineculau merged commit 2bc49b4 into master [83]about 2 years ago
andreineculau deleted the f/anu-merge-aws-cfn-util-firecloud branch

[84]about 2 years ago

simplify `src/lambda.js:_setupLogging` #13
Closed

[57]andreineculau opened this issue Dec 10, 2018 · 0 comments

Closed
[58]simplify src/lambda.js:_setupLogging #13
[59]andreineculau opened this issue Dec 10, 2018 · 0 comments

Assignees

[60]@andreineculau

Labels

[61]enhancement
Comments
[62]@andreineculau

Copy link (BUTTON) Quote reply

Member
[63]@andreineculau [64]andreineculau commented [65]Dec 10, 2018 •
edited
by removing the heavy bunyan (and bunyan-slack), and replacing with

[66]https://github.com/tobiipro/minlog
Why? I see webpack.config.jsfiles having exceptions for bunyan's

dtrace-provider dependency, plus dtrace-provider has native bindings,

thus heavy on its own.

[67]@andreineculau [68]andreineculau added the [69]enhancement label

[70]Dec 10, 2018

[71]@andreineculau [72]andreineculau self-assigned this [73]Dec 10,

2018

[74]@andreineculau [75]andreineculau mentioned this issue [76]Feb 4,

2019

[77]break up lambda module, replace bunyan with minlog #16

Merged

1 of 1 task complete

[78]@andreineculau [79]andreineculau closed this [80]Feb 4, 2019

maybe simplify access to e, ctx, ctx.log #20
Closed

[57]andreineculau opened this issue Feb 20, 2019 · 2 comments

Closed
[58]maybe simplify access to e, ctx, ctx.log #20
[59]andreineculau opened this issue Feb 20, 2019 · 2 comments

Labels

[60]question [61]wontfix
Comments
[62]@andreineculau

Copy link (BUTTON) Quote reply

Member
[63]@andreineculau [64]andreineculau commented [65]Feb 20, 2019
rather than passing e.g. ctx around, just to able to call

ctx.log.error, given the lambda's serial execution flow (one event at a

time), maybe we could do

import {e, ctx, log} from 'aws-util-firecloud/lib/lambda';
log.error(...)
wdyt [66]@IanSavchenko ?

[67]@andreineculau [68]andreineculau added the [69]question label

[70]Feb 20, 2019

[71]@IanSavchenko
This comment has been minimized.
[72]Sign in to view

Copy link (BUTTON) Quote reply

Contributor
[73]@IanSavchenko [74]IanSavchenko commented [75]Feb 20, 2019
I would not do this, looks like not a good pattern to me. And it will

make it harder to test.

[76]@andreineculau
This comment has been minimized.
[77]Sign in to view

Copy link (BUTTON) Quote reply

Member Author
[78]@andreineculau [79]andreineculau commented [80]Feb 21, 2019
decided IRL to only do it for log, and as a global var

[81]@andreineculau [82]andreineculau added this to To do in [83]Public

(Open Source) [84]May 20, 2019

[85]@andreineculau [86]andreineculau added the [87]wontfix label

[88]Jul 9, 2019

[89]@andreineculau [90]andreineculau closed this [91]Jul 9, 2019

[92]Public (Open Source) automation moved this from To do to Done

[93]Jul 9, 2019

[94]@ankitmth [95]ankitmth removed this from Done in [96]Public (Open

Source) [97]Feb 19, 2020

Athena: convert query result to object using column type information #4
Closed

[57]IanSavchenko opened this issue Mar 12, 2018 · 1 comment

Closed
[58]Athena: convert query result to object using column type information #4
[59]IanSavchenko opened this issue Mar 12, 2018 · 1 comment

Assignees

[60]@IanSavchenko
Comments
[61]@IanSavchenko

Copy link (BUTTON) Quote reply

Contributor
[62]@IanSavchenko [63]IanSavchenko commented [64]Mar 12, 2018 •
edited
[65]https://docs.aws.amazon.com/athena/latest/ug/data-types.html

[66]https://prestodb.io/docs/current/language/types.html

[67]@IanSavchenko [68]IanSavchenko self-assigned this [69]Mar 12, 2018

[70]@IanSavchenko
This comment has been minimized.
[71]Sign in to view

Copy link (BUTTON) Quote reply

Contributor Author
[72]@IanSavchenko [73]IanSavchenko commented [74]Mar 14, 2018
Fixed with [75]efd1013

[76]@IanSavchenko [77]IanSavchenko closed this [78]Mar 14, 2018

silence errors when failing to get lambda checksums #17
f/anu-no-checksum
master f/anu-no-checksum

@andreineculau

[45]andreineculau opened this pull request

about 1 year ago
Labels

[46]enhancement

No description given.
[47]andreineculau added a commit [48]about 1 year ago

[49]@andreineculau [50]silence errors when failing to get lambda

checksums
andreineculau added the [51]enhancement label [52]about 1 year ago
andreineculau self-assigned this [53]about 1 year ago
andreineculau requested a review from IanSavchenko [54]about 1 year ago
@IanSavchenko

[55]IanSavchenko approved these changes [56]about 1 year ago
andreineculau referenced this pull request from commit [57]1b8487c

[58]about 1 year ago
andreineculau merged commit 1b8487c into master [59]about 1 year ago
andreineculau deleted the f/anu-no-checksum branch [60]about 1 month

ago

Clean up and clarify S3 "get...Bucket" functions to have correct semantics #9
Closed

[57]IanSavchenko opened this issue Apr 26, 2018 · 1 comment

Closed
[58]Clean up and clarify S3 "get...Bucket" functions to have correct

semantics #9
[59]IanSavchenko opened this issue Apr 26, 2018 · 1 comment

Assignees

[60]@IanSavchenko
Comments
[61]@IanSavchenko

Copy link (BUTTON) Quote reply

Contributor
[62]@IanSavchenko [63]IanSavchenko commented [64]Apr 26, 2018
No description provided.

[65]@IanSavchenko [66]IanSavchenko self-assigned this [67]May 30, 2018

[68]@IanSavchenko
This comment has been minimized.
[69]Sign in to view

Copy link (BUTTON) Quote reply

Contributor Author
[70]@IanSavchenko [71]IanSavchenko commented [72]May 31, 2018
Fixed with [73]8c74b49

[74]@IanSavchenko [75]IanSavchenko closed this [76]May 31, 2018

reuse code for aws-sdk custom logger #33
reuse-logger
master reuse-logger

@andreineculau

[45]andreineculau opened this pull request

9 months ago
Labels

[46]enhancement

* Fixes:

* Breaking change: [ ]

__________________________________________________________________
[47]andreineculau added a commit [48]9 months ago

[49]@andreineculau [50]reuse code for aws-sdk custom logger
andreineculau added the [51]enhancement label [52]9 months ago
andreineculau requested a review from IanSavchenko [53]9 months ago
andreineculau self-assigned this [54]9 months ago
@IanSavchenko

[55]IanSavchenko approved these changes [56]9 months ago
IanSavchenko referenced this pull request from commit [57]8584a77 [58]9

months ago
IanSavchenko merged commit 8584a77 into master [59]9 months ago
andreineculau referenced this pull request from commit [60]0853afc

[61]7 months ago
andreineculau deleted the reuse-logger branch [62]about 1 month ago

Add 'has' function to env proxy and start using it in lambda bootstrap #7
Closed

[57]IanSavchenko opened this issue Mar 27, 2018 · 1 comment

Closed
[58]Add 'has' function to env proxy and start using it in lambda bootstrap #7
[59]IanSavchenko opened this issue Mar 27, 2018 · 1 comment
Comments
[60]@IanSavchenko

Copy link (BUTTON) Quote reply

Contributor
[61]@IanSavchenko [62]IanSavchenko commented [63]Mar 27, 2018
No description provided.

[64]@andreineculau [65]andreineculau mentioned this issue [66]Mar 14,

2019

[67]breaking: remove overloaded _ property from safeProxy; prefer 'in'

ch… #17

Merged

[68]@andreineculau
This comment has been minimized.
[69]Sign in to view

Copy link (BUTTON) Quote reply

Member
[70]@andreineculau [71]andreineculau commented [72]May 20, 2019
fixed by [73]tobiipro/lodash-firecloud#17

[74]@andreineculau [75]andreineculau closed this [76]May 20, 2019

breaking: remove deprecated url.parse/format usage #24
f/anu-remove-deprecated-url
master f/anu-remove-deprecated-url

@andreineculau

[45]andreineculau opened this pull request

about 1 year ago
Labels

[46]enhancement

* Fixes:

* Breaking change: [x]

__________________________________________________________________
andreineculau added the [47]enhancement label [48]about 1 year ago
andreineculau self-assigned this [49]about 1 year ago
andreineculau requested a review from IanSavchenko [50]about 1 year ago
@IanSavchenko

[51]IanSavchenko approved these changes [52]about 1 year ago
[53]andreineculau added some commits [54]about 1 year ago

[55]@andreineculau [56]updated support-firecloud

[57]@andreineculau [58]upgrade jest

[59]@andreineculau [60]breaking: remove deprecated url.parse/format

usage
andreineculau force pushed changes to this branch [61]about 1 year ago
andreineculau merged commit 8a61994 into master [62]about 1 year ago
andreineculau deleted the f/anu-remove-deprecated-url branch [63]about

1 month ago

remove url #26
Closed

[57]andreineculau opened this issue Mar 14, 2019 · 1 comment

Closed
[58]remove url #26
[59]andreineculau opened this issue Mar 14, 2019 · 1 comment

Assignees

[60]@andreineculau

Labels

[61]enhancement
Comments
[62]@andreineculau

Copy link (BUTTON) Quote reply

Member
[63]@andreineculau [64]andreineculau commented [65]Mar 14, 2019
i.e. revert the [66]96889a2 revert (emergency fix)

[67]@andreineculau [68]andreineculau added the [69]enhancement label

[70]Mar 14, 2019

[71]@andreineculau [72]andreineculau self-assigned this [73]Mar 14,

2019

[74]@andreineculau [75]andreineculau added this to To do in [76]Public

(Open Source) [77]May 20, 2019

[78]@andreineculau
This comment has been minimized.
[79]Sign in to view

Copy link (BUTTON) Quote reply

Member Author
[80]@andreineculau [81]andreineculau commented [82]Jul 12, 2019
done in [83]cca3f39

[84]@andreineculau [85]andreineculau closed this [86]Jul 12, 2019

[87]Public (Open Source) automation moved this from To do to Done

[88]Jul 12, 2019

[89]@ankitmth [90]ankitmth removed this from Done in [91]Public (Open

Source) [92]Feb 19, 2020

[ATEX-283] add test for firehose.putRecords #1
atex-283
master atex-283

@andreineculau

[45]andreineculau opened this pull request

about 2 years ago
A few basic tests primarily directed at [46]Firehose limitations.
andreineculau requested a review from IanSavchenko [47]about 2 years

ago
andreineculau force pushed changes to this branch [48]about 2 years ago
andreineculau force pushed changes to this branch [49]about 2 years ago
[50]andreineculau added a commit [51]about 2 years ago

[52]@andreineculau [53][ATEX-283] add test for firehose.putRecords
andreineculau force pushed changes to this branch [54]about 2 years ago
[55]andreineculau added some commits [56]about 2 years ago

[57]@andreineculau [58]group firehose limits

[59]@andreineculau [60]style
@IanSavchenko

[61]IanSavchenko reviewed [62]about 2 years ago

[63]test/firehose.test.js

((8 lines not shown))

};

// + 1 for the new line character

let byteSizeOverhead = Buffer.byteLength(JSON.stringify({content: ''}))

1;

describe('firehose', function() {

describe('putRecords', function() {

test('should call _putRecordBatches', async function() {

let byteSize = 25 - byteSizeOverhead;

let records = _.times(1, function() {

return {

content: generate({byteSize})

};

});

let spy = jest.spyOn(firehose, '_putRecordBatches')

[64]@IanSavchenko [65]IanSavchenko • [66]about 2 years ago • edited

about 2 years ago

Copy link

This should work, of course, but I don't think you are supposed to

write unit tests based on "private" members invocations or

implementation details. Why not spy on aws-sdk methods? It's amazon

limits we are trying not to break, not ours.

[67]@andreineculau [68]andreineculau • [69]about 2 years ago

Copy link
 I don't think you are supposed to write unit tests based on
 "private"

What are you saying here? That you are of the strong opinion that

private functions should not be unit tested, or that you've read/heard

that you're not supposed to do that? The former I'm interested in

pursuing, the latter 🤷‍♂ 😉
Indirect testing has a conceptual flaw in the context of unit testing,

in that the black box boundaries become subjectively defined. Example:

since I'm interested in whether the public interface works or not, then

why mock aws-sdk rather than using it as is? Because it's external

code? Because someone else should have tested that? Because it requires

external resources? Because its functionality is dependant on

hardware/runtime factors? And if _putRecordBatches becomes public, then

what? Ok or not ok to mock it to test putRecords? And if I'm using

lodash utils? Should I mock those or not?
In this specific case, I mocked _putRecordBatches because

1. it's an easier MVP (as in mocking aws-sdk will probably end up in

creating an module-level mock since there is more code in this repo

that has aws-sdk as exitpoint, and then each function has callback

signature by default, etc)

2. _putRecordBatches could easily become putRecordBatches 🎉
 It's amazon limits we are trying not to break, not ours.

Incorrect imo. I'm testing that a function (the unit) does the right

thing given the expectation and the constraints. I could for instance

have the byteSize of a batch further limited based on available memory.

And for lets-be-reasonable reasons, I'm mocking anything that cannot

run locally e.g. if some call to aws-sdk would not be influenced by

environment or external calls (like getting a hardcoded list of known

aws regions), then I wouldn't mock it.

[70]@andreineculau [71]andreineculau • [72]about 2 years ago

Copy link
 Indirect testing has a conceptual flaw in the context of unit
 testing, in that the black box boundaries become subjectively
 defined.

For clarity, my argument is that arguing about the boundaries of

indirect testing is void of essence. Not that there should be no

indirect testing (i.e. mock everything) - on the contrary.

[73]@IanSavchenko [74]IanSavchenko • [75]about 2 years ago

Copy link
Wow-wow-wow! Easier :)

1. It's my opinion regarding "private" methods unit testing. I don't

care how it works and should feel free to remove/rename/change

"private" methods as I want while side-effects and the output are

the same.

2. I don't see any problem with mocking aws-sdk at all, since you pass

'firehose' instance in the context.

3. Again: it does not matter how you call internal methods in the

module, there are no restrictions (almost). Only 2 things are

important: firehose.putRecordBatch() should not receive too big

data and the input (if it meets the limits) should be correct. How

these goals are met - does not matter.

4. Not saying you should mock anything at all. If you can track how

firehose sdk is invoked, don't mock it, just wrap/proxy/do-whatever

with it.
I don't want to argue, we have very different views on this and starts

to get deep into conceptual talks. So, I step away 🚪
Copy link

@IanSavchenko

[76]IanSavchenko

commented [77]about 2 years ago
Nice work! 👍
IanSavchenko closed this pull request [78]about 2 years ago
IanSavchenko reopened this pull request [79]about 2 years ago
Copy link

@IanSavchenko

[80]IanSavchenko

commented [81]about 2 years ago
Oh, go ahead merge it 👍
@IanSavchenko

[82]IanSavchenko approved these changes [83]about 2 years ago
andreineculau referenced this pull request from commit [84]a3dbd8e

[85]about 2 years ago
andreineculau merged commit a3dbd8e into master [86]about 2 years ago
andreineculau deleted the atex-283 branch [87]about 2 years ago

Athena pagination #30
athena-pagination
master athena-pagination

@IanSavchenko

[45]IanSavchenko opened this pull request

10 months ago
Linked issues

[46]Add pagination support for athena module

* Fixes: [47]#29

* Breaking change: [ ]

__________________________________________________________________
Added pagination for high-level function executeQuery. By default, it

will allow 10000 rows, but configurable.
SIDENOTE: Also, I would like to improve a bit the API for athena

functions, make it tighter, but it will be a breaking change.
Ian Savchenko added a commit [48]10 months ago

[49]athena utils: add pagination
IanSavchenko requested a review from andreineculau [50]10 months ago
IanSavchenko requested a review from fraser-campbell [51]10 months ago
Ian Savchenko added a commit [52]10 months ago

[53]handle column types more safely
IanSavchenko force pushed changes to this branch [54]10 months ago
@andreineculau

[55]andreineculau reviewed [56]10 months ago
since you have anyway introduced the concept of maxRows, I would say

just add a breakCb argument, and let the caller decide when to stop

function(rows) {

if (rows.length > 10000) return true;

}
(BUTTON) Show outdated (BUTTON) Hide outdated [57]src/athena.js

((18 lines not shown))

try {

if (.isDefined(value)) {

switch (.toLower(column.Type)) {

case ('integer'):

case ('tinyint'):

case ('smallint'):

case ('bigint'):

case ('double'):

value = Number(value);

break;

case ('boolean'):

value = Boolean(value);

break;

default:

break;

}

[58]@andreineculau [59]andreineculau • [60]10 months ago

Copy link
why try catch?

not enough to use _.toNumber ?
granted I don't know what you get when 'boolean' . there's no

_.toBoolean [61]lodash/lodash#2066

[62]@IanSavchenko [63]IanSavchenko • [64]10 months ago

Copy link
The reason I added this is primarily for column names, which will

always be strings in the first row (while column values could be

something else, but we haven't had such examples on practice). To fix

it more properly, I would have to drop column names row before passing

it into this function and then it would require change of the API,

because right now queryResultToObjectsArray is not the best with

accepting queryResult, while it's better to accept

queryResult.ResultSet (which I would like to do, but separately, I

guess).
That's why I decided to make a safe fallback to strings.

(BUTTON) Show outdated (BUTTON) Hide outdated [65]src/athena.js

@@ -139,7 +143,8 @@ export let executeQuery = async function({

}

},

pollingDelay = 1000,

initPollingDelay = pollingDelay

initPollingDelay = pollingDelay,

maxRows = 10000

[66]@andreineculau [67]andreineculau • [68]10 months ago

Copy link
shouldn't we start with a really low max (like 10), so as to force

users to understand what they're doing?

[69]@IanSavchenko [70]IanSavchenko • [71]10 months ago

Copy link
Giving a very low number would be a breaking change. I guess a more

safe option would be to use Athena's default (1000 and they decided

that it's fine). As I said, I want this function to be very simple for

the user, so I would give a safely big number and only if you need

more, you can tweak it.

(BUTTON) Show resolved (BUTTON) Hide resolved [72]src/athena.js

((10 lines not shown))

if (queryResultIsShowResult(queryResult)) {

return queryResultToText(queryResult);

}

// checking if the query is a result of SHOW query

// returing just text in this case, not trying to parse columns and

rows

if (queryResultIsShowResult(queryResult)) {

return queryResultToText(queryResult);

}

nextToken = queryResult.NextToken;

[73]@andreineculau [74]andreineculau • [75]10 months ago

Copy link
why not treat pagination as separate case?

* get results

* is this SHOW? then return

* are there more rows? then get those as well
[76]@IanSavchenko [77]IanSavchenko • [78]10 months ago

Copy link
It could be done like that, but then it will be a bit of code

duplication. I don't have much preference with that, so if you

insist...

(BUTTON) Show outdated (BUTTON) Hide outdated [79]src/athena.js

((10 lines not shown))

if (queryResultIsShowResult(queryResult)) {

return queryResultToText(queryResult);

}

// checking if the query is a result of SHOW query

// returing just text in this case, not trying to parse columns and

rows

if (queryResultIsShowResult(queryResult)) {

return queryResultToText(queryResult);

}

nextToken = queryResult.NextToken;

rows = rows.concat(queryResultToObjectsArray(queryResult));

} while (nextToken && rows.length <= maxRows);

[80]@andreineculau [81]andreineculau • [82]10 months ago

Copy link
don't you mean maxRows + 1 (column names) ?

[83]@andreineculau [84]andreineculau • [85]10 months ago

Copy link
or actually to _.drop(rows, 1) immediately after getting the first page

(if you split the code like I wrote above)

[86]@IanSavchenko [87]IanSavchenko • [88]10 months ago

Copy link
 maxRows + 1

that's why there is rows.length <= maxRows instead of rows.length <

maxRows + 1. But I agree that it could be cleaner.
andreineculau added this to Review in progress in [89]Public (Open

Source) [90]10 months ago
IanSavchenko was assigned by andreineculau [91]10 months ago
Copy link

@IanSavchenko

[92]IanSavchenko

commented [93]10 months ago
 since you have anyway introduced the concept of maxRows, I would say
 just add a breakCb argument, and let the caller decide when to stop

function(rows) {

if (rows.length > 10000) return true;

}
I want to make it super simple: here is a query, give me results. I see

as the user for this, for example, someone like Cory and while I don't

hesitate that she can master callbacks (or maybe she already has),

passing a callback is quite advanced (as to me), and maxRows on a

contrary is a clear and common way to work with such APIs.
Ian Savchenko added some commits [94]10 months ago

[95]Revert "handle column types more safely"

[96]remove maxRows param
IanSavchenko requested a review from andreineculau [97]10 months ago
@andreineculau

[98]andreineculau approved these changes [99]10 months ago
Public (Open Source) automation moved this from Review in progress to

Reviewer approved [100]10 months ago
IanSavchenko referenced this pull request from commit [101]8cf1bda

[102]10 months ago
IanSavchenko merged commit 8cf1bda into master [103]10 months ago
Public (Open Source) automation moved this from Reviewer approved to

Done [104]10 months ago
IanSavchenko deleted the athena-pagination branch [105]10 months ago
ankitmth removed this from Done in [106]Public (Open Source) [107]17

days ago

better errors for cases when Lambda build artifacts not found #14
ian-addLambda-notFound
master ian-addLambda-notFound

@IanSavchenko

[45]IanSavchenko opened this pull request

about 1 year ago

No description given.
Ian Savchenk... added a commit [46]about 1 year ago

[47]better errors for cases when build artifacts not found
IanSavchenko requested a review from andreineculau [48]about 1 year ago
@andreineculau

[49]andreineculau approved these changes [50]about 1 year ago
andreineculau referenced this pull request from commit [51]2f3babb

[52]about 1 year ago
andreineculau merged commit 2f3babb into master [53]about 1 year ago
Copy link

@andreineculau

[54]andreineculau

commented [55]about 1 year ago
technically there's no reason to hide the getCodeChecksum function,

right?
IanSavchenko deleted the ian-addLambda-notFound branch [56]about 1 year

ago
Copy link

@IanSavchenko

[57]IanSavchenko

commented [58]about 1 year ago
I saw it is not used anywhere outside, so having it private puts less

responsibility on backward compatibility.

kinesis put records #6
f/kinesis-put-records
master f/kinesis-put-records

@andreineculau

[45]andreineculau opened this pull request

almost 2 years ago
wip. needs tests

don't know if it's worth trying to make this and firehose.putRecords

abstract
Copy link

@IanSavchenko

[46]IanSavchenko

commented [47]almost 2 years ago
Abstract in a sense to be reused between these two? I don't think it

should, it's just a coincidence that they have same (or just similar?)

API and there might be more differences later.

❤️ 1

andreineculau reacted with heart emoji
andreineculau force pushed changes to this branch [48]over 1 year ago
[49]andreineculau added some commits [50]over 1 year ago

[51]@andreineculau [52]less assumptions about firehose in test

[53]@andreineculau [54]add putRecords functionality to kinesis
andreineculau force pushed changes to this branch [55]over 1 year ago
Copy link

@andreineculau

[56]andreineculau

commented [57]over 1 year ago
[58]@IanSavchenko refreshed the PR, just in case you're ok merging it
@IanSavchenko

[59]IanSavchenko reviewed [60]about 1 year ago
Nice! Sorry, it took so long to review it, but I wanted to be focused

when doing it.

[61]src/kinesis.js

((38 lines not shown))

Records: [],

byteSize: 0

};

let toProcessCount = records.length;

_.forEach(records, function(record) {

let Data = JSON.stringify(record);

let dataLength = Buffer.byteLength(JSON.stringify({

Data: record,

ExplicitHashKey,

PartitionKey

}));

if (dataLength > limits.recordByteSize) {

ctx.log.error(`Skipping record larger than ${limits.recordByteSize /

1024} KB: 

[62]@IanSavchenko [63]IanSavchenko • [64]about 1 year ago

Copy link
I'm questioning now this approach where a lib function almost silently

fails to send records. I know we have it in the firehose too, but

still. I would say it's up to the app to decide how to deal with such

records.
Should we maybe just expose a helper function that checks a record size

and allows a client to decide what to do with non-conforming records?

[65]@andreineculau [66]andreineculau • [67]about 1 year ago

Copy link
Sounds good! But as part of another PR
andreineculau referenced this pull request from commit [68]2f76e70

[69]about 1 year ago
andreineculau merged commit 2f76e70 into master [70]about 1 year ago
[71]andreineculau referenced this pull request from another pull

request

[72]#11 allow control over large records on kinesis/firehose
andreineculau deleted the f/kinesis-put-records branch [73]about 1 year

ago

feat: log if event loop is not empty when the lambda responds #25
Closed

[57]andreineculau opened this issue Mar 11, 2019 · 4 comments

Closed
[58]feat: log if event loop is not empty when the lambda responds #25
[59]andreineculau opened this issue Mar 11, 2019 · 4 comments

Labels

[60]enhancement
Comments
[61]@andreineculau

Copy link (BUTTON) Quote reply

Member
[62]@andreineculau [63]andreineculau commented [64]Mar 11, 2019
we had an instance where the lambda logged a response to AWS, but then

it was waiting for the event loop to become empty, which never

happened, thus the lambda timed out (and the initial response was

ignored)
it would be good to at least log that "hey we are now replying to AWS.

but this lambda is configured to wait until the event loop is empty,

and there's smth on the event loop right now" - even better if we can

report some useful info about what is actually on the event loop
currently there are only some undocumented nodejs APIs -

process._getActiveRequests() and process._getActiveHandles() - and even

those don't work as expected. I tried in a node REPL to run a simple

async function that sleeps for 10 seconds then console.log-s smth and

process._getActiveRequests() remained empty, while I was expecting it

to report the async function somehow.

[65]@andreineculau [66]andreineculau added the [67]enhancement label

[68]Mar 11, 2019

[69]@andreineculau
This comment has been minimized.
[70]Sign in to view

Copy link (BUTTON) Quote reply

Member Author
[71]@andreineculau [72]andreineculau commented [73]Mar 11, 2019
tried now also with a setTimeout - still empty array result from

process._getActiveRequests()

[74]@IanSavchenko
This comment has been minimized.
[75]Sign in to view

Copy link (BUTTON) Quote reply

Contributor
[76]@IanSavchenko [77]IanSavchenko commented [78]Mar 25, 2019
This worked for me [79]https://github.com/mafintosh/why-is-node-running

let log = require('why-is-node-running');
setTimeout(function() {

console.log('done!');

}, 2000);
log();
Outputs

There are 1 handle(s) keeping the process running
Timeout
/Users/isao/code/github/TobiiPro/atex-platform/test.js:3 - setTimeout(function()

{
(HEART react) ❤️ 1

[80]@andreineculau
This comment has been minimized.
[81]Sign in to view

Copy link (BUTTON) Quote reply

Member Author
[82]@andreineculau [83]andreineculau commented [84]Mar 25, 2019
👏 I thought async_hooks came in nodejs 10, but i see now it's also in

nodejs 8
don't know if it's in the version that aws delivers, but it's worth a

try

[85]@andreineculau [86]andreineculau added this to To do in [87]Public

(Open Source) [88]May 20, 2019

[89]andreineculau added a commit to tobiipro/lodash-firecloud that

referenced this issue [90]Aug 6, 2019

[91]@andreineculau

[92]wip: add mixins around stacktrace and open handles. ref

[93]tobiipro/aws-… (BUTTON) …

Verified

This commit was signed with a verified signature.

[94]andreineculau Andrei Neculau

GPG key ID: 79FA7EE650BF9A61 [95]Learn about signing commits

[96]0bc8791

[97]…util-firecloud#25
[98]@andreineculau [99]andreineculau moved this from To do to In

progress in [100]Public (Open Source) [101]Aug 6, 2019

[102]andreineculau added a commit to tobiipro/lodash-firecloud that

referenced this issue [103]Aug 7, 2019

[104]@andreineculau

[105]add mixins around stacktrace and open handles. ref

[106]tobiipro/aws-util-… (BUTTON) …

Verified

This commit was signed with a verified signature.

[107]andreineculau Andrei Neculau

GPG key ID: 79FA7EE650BF9A61 [108]Learn about signing commits

[109]157e767

[110]…firecloud#25
[111]andreineculau added a commit to tobiipro/lodash-firecloud that

referenced this issue [112]Aug 7, 2019

[113]@andreineculau

[114]add mixins around stacktrace and open handles. ref

[115]tobiipro/aws-util-… (BUTTON) …

Verified

This commit was signed with a verified signature.

[116]andreineculau Andrei Neculau

GPG key ID: 79FA7EE650BF9A61 [117]Learn about signing commits

[118]55f6fca

[119]…firecloud#25
[120]andreineculau added a commit to tobiipro/lodash-firecloud that

referenced this issue [121]Aug 7, 2019

[122]@andreineculau

[123]add mixins around stacktrace and open handles. ref

[124]tobiipro/aws-util-… (BUTTON) …

Verified

This commit was signed with a verified signature.

[125]andreineculau Andrei Neculau

GPG key ID: 79FA7EE650BF9A61 [126]Learn about signing commits

[127]913a73b

[128]…firecloud#25
[129]andreineculau added a commit to tobiipro/lodash-firecloud that

referenced this issue [130]Aug 7, 2019

[131]@andreineculau

[132]add mixins around stacktrace and open handles. ref

[133]tobiipro/aws-util-… (BUTTON) …

Verified

This commit was signed with a verified signature.

[134]andreineculau Andrei Neculau

GPG key ID: 79FA7EE650BF9A61 [135]Learn about signing commits

[136]574caf1

[137]…firecloud#25
[138]@andreineculau
This comment has been minimized.
[139]Sign in to view

Copy link (BUTTON) Quote reply

Member Author
[140]@andreineculau [141]andreineculau commented [142]Aug 16, 2019
closing without new functionality in aws-util-firecloud. when a lambda

times out and needs investigation, one can wrap the lambda and diagnose

with the help of _.getV8OpenHandles()

[143]@andreineculau [144]andreineculau closed this [145]Aug 16, 2019

[146]Public (Open Source) automation moved this from In progress to

Done [147]Aug 16, 2019

[148]@ankitmth [149]ankitmth removed this from Done in [150]Public

(Open Source) [151]Feb 19, 2020

allow control over large records on kinesis/firehose #11
f/anu-control-large-records
master f/anu-control-large-records

@andreineculau

[45]andreineculau opened this pull request

about 1 year ago
Labels

[46]enhancement
from [47]#6 (comment)
andreineculau added the [48]enhancement label [49]about 1 year ago
andreineculau requested a review from IanSavchenko [50]about 1 year ago
@IanSavchenko

[51]IanSavchenko reviewed [52]about 1 year ago
So instead of moving out control over oversized packages from lib

functions, the function becomes even more complicated and takes extra

params. I don't understand what kind of logic you expect to be in

onLargeRecordEntries to happen so it can decide if we should continue

processing. It should either fail or ignore and a boolean flag would be

enough to do that.
Copy link

@andreineculau

[53]andreineculau

commented [54]about 1 year ago
no need for the cynicism. That's just how I interpreted your comment

[55]#6 (comment) . And to the point, it sounds like you understand the

logic of onLargeRecordEntries just fine 😉
Back to your initial suggestion, having a helper function to check a

record size (or record sizes of a multiple records means that you run

JSON.stringify twice over possibly large records). An alternative would

be a prep function that would return "recordBatches AND

tooLargeRecords". The caller could then decide what to do with the

tooLargeRecords, and maybe forward recordBatches to a putRecordBatches

function.
Copy link

@IanSavchenko

[56]IanSavchenko

commented [57]about 1 year ago
I wanted to (re)move out code, not add more code 😄
Yes, I wanted to run serialization twice, but make logic more simple. I

don't think that it will be a huge performance hit and it's not like we

are fighting for performance here. Also, I expect 90% of the records to

be small, so it won't be that critical either.
From a performance perspective, it can be optimized in a way that:

1. we make it in a way that function accepts only strings (stringified

records)

2. we can pre-check size in bytes only for those stringified records

that have bigger length (like 500k)
andreineculau force pushed changes to this branch [58]about 1 year ago
Copy link

@andreineculau

[59]andreineculau

commented [60]about 1 year ago
there's performance and performance... stringifying 500 records of 1MB

each = 6.5 seconds in a simple local test.
If we say we don't care about backward compat, we can do like you say

in 2 steps. It has the added benefit that the putRecords functions

become agnostic of serialization. The downside is that now you'll need

two calls ourFirehose.recordsToJSON + ourFirehose.putRecords but you

can't have the cake and eat it too.
Otherwise, a middle ground would be to at least return the records that

were skipped. The app could decide what to do with them. (updated the

PR with this, not saying that I favour it, just that it was easy to do

it)
@IanSavchenko

[61]IanSavchenko approved these changes [62]about 1 year ago
Probably, the right solution would be to check the size of every record

in the batch and fail early (or ignore them, depending on a flag) if

some of them are oversized, even before attempting to send any record.

In this way, we would provide some integrity of operation (maybe).
But currently, it's good enough.
[63]andreineculau added a commit [64]about 1 year ago

[65]@andreineculau [66]allow control over large records on

kinesis/firehose
andreineculau force pushed changes to this branch [67]about 1 year ago
Copy link

@andreineculau

[68]andreineculau

commented [69]about 1 year ago
 Probably, the right solution would be to check the size of every
 record in the batch and fail early (or ignore them, depending on a
 flag) if some of them are oversized, even before attempting to send
 any record. In this way, we would provide some integrity of
 operation (maybe).

Hmm, that sounds so similar to... something... I know! my first

iteration 😝 the only diff: change callback to flag

😄 1

IanSavchenko reacted with laugh emoji
andreineculau referenced this pull request from commit [70]50d3c39

[71]about 1 year ago
andreineculau merged commit 50d3c39 into master [72]about 1 year ago
andreineculau deleted the f/anu-control-large-records branch [73]about

1 year ago

add typescript definition #31
Closed

[57]andreineculau opened this issue May 20, 2019 · 0 comments · Fixed

by [58]#35

Closed
[59]add typescript definition #31
[60]andreineculau opened this issue May 20, 2019 · 0 comments · Fixed

by [61]#35

Labels

[62]enhancement
Comments
[63]@andreineculau

Copy link (BUTTON) Quote reply

Member
[64]@andreineculau [65]andreineculau commented [66]May 20, 2019
see lodash-firecloud setup

[67]@andreineculau [68]andreineculau added the [69]enhancement label

[70]May 20, 2019

[71]@andreineculau [72]andreineculau added this to To do in [73]Public

(Open Source) via automation [74]May 20, 2019

[75]@andreineculau [76]andreineculau mentioned this issue [77]Nov 21,

2019

[78]Typescript #35

Merged

[79]@andreineculau [80]andreineculau closed this in [81]#35 [82]Jan 18,

2020

[83]Public (Open Source) automation moved this from To do to Done

[84]Jan 18, 2020

[85]@ankitmth [86]ankitmth removed this from Done in [87]Public (Open

Source) [88]Feb 19, 2020

Add pagination support for athena module #29
Closed

[57]IanSavchenko opened this issue May 13, 2019 · 0 comments · Fixed by

[58]#30

Closed
[59]Add pagination support for athena module #29
[60]IanSavchenko opened this issue May 13, 2019 · 0 comments · Fixed by

[61]#30

Assignees

[62]@IanSavchenko

Labels

[63]enhancement
Comments
[64]@IanSavchenko

Copy link (BUTTON) Quote reply

Contributor
[65]@IanSavchenko [66]IanSavchenko commented [67]May 13, 2019
When reading results, we should support the possibility that the data

has pages.

[68]@IanSavchenko [69]IanSavchenko added the [70]enhancement label

[71]May 13, 2019

[72]@IanSavchenko [73]IanSavchenko self-assigned this [74]May 13, 2019

[75]@IanSavchenko [76]IanSavchenko mentioned this issue [77]May 16,

2019

[78]Athena pagination #30

Merged

[79]@andreineculau [80]andreineculau added this to In progress in

[81]Public (Open Source) [82]May 20, 2019

[83]@IanSavchenko [84]IanSavchenko closed this in [85]#30 [86]May 27,

2019

[87]Public (Open Source) automation moved this from In progress to Done

[88]May 27, 2019

[89]@ankitmth [90]ankitmth removed this from Done in [91]Public (Open

Source) [92]Feb 19, 2020

replace bunyan with minlog #5
Closed

[57]andreineculau opened this issue Mar 14, 2018 · 0 comments

Closed
[58]replace bunyan with minlog #5
[59]andreineculau opened this issue Mar 14, 2018 · 0 comments

Labels

[60]enhancement
Comments
[61]@andreineculau

Copy link (BUTTON) Quote reply

Member
[62]@andreineculau [63]andreineculau commented [64]Mar 14, 2018
No description provided.

[65]@andreineculau [66]andreineculau added the [67]enhancement label

[68]Mar 14, 2018

[69]@andreineculau [70]andreineculau mentioned this issue [71]Feb 4,

2019

[72]break up lambda module, replace bunyan with minlog #16

Merged

1 of 1 task complete

[73]@andreineculau [74]andreineculau closed this [75]Feb 4, 2019

F/anu better inspect #22
f/anu-better-inspect
master f/anu-better-inspect

@andreineculau

[45]andreineculau opened this pull request

about 1 year ago
Labels

[46]enhancement

* Fixes:

* Breaking change: [ ]

__________________________________________________________________
a bit of refactoring and adding cpu-usage
andreineculau added the [47]enhancement label [48]about 1 year ago
[49]andreineculau added a commit [50]about 1 year ago

[51]@andreineculau [52]refactor inspect
andreineculau force pushed changes to this branch [53]about 1 year ago
andreineculau referenced this pull request from commit [54]3f27e67

[55]about 1 year ago
andreineculau merged commit 3f27e67 into master [56]about 1 year ago
andreineculau deleted the f/anu-better-inspect branch [57]about 1 month

ago

upgrade minlog, and implement global await flush() #36
minlog-flush
master minlog-flush

@andreineculau

[45]andreineculau opened this pull request

4 months ago
Labels

[46]enhancement

* Fixes:

* Breaking change: [x]

__________________________________________________________________
[47]andreineculau added a commit [48]4 months ago

[49]@andreineculau [50]upgrade minlog, and implement global await

flush()
andreineculau added the [51]enhancement label [52]4 months ago
andreineculau self-assigned this [53]4 months ago
andreineculau added this to In progress in [54]Public (Open Source) via

automation [55]4 months ago
andreineculau referenced this pull request from commit [56]9a3e7e2

[57]3 months ago
andreineculau merged commit 9a3e7e2 into master [58]3 months ago
Public (Open Source) automation moved this from In progress to Done

[59]3 months ago
andreineculau deleted the minlog-flush branch [60]3 months ago
ankitmth removed this from Done in [61]Public (Open Source) [62]17 days

ago