Skip to content

Conversation

estolfo
Copy link
Contributor

@estolfo estolfo commented Feb 23, 2022

This PR addresses some comments from this code review: #124 (comment)

Todo (3 March)

  • Use JSON.Decoder

@github-actions github-actions bot added the aws-λ-extension AWS Lambda Extension label Feb 23, 2022
@ghost
Copy link

ghost commented Feb 23, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-03-09T10:53:11.422+0000

  • Duration: 7 min 38 sec

Test stats 🧪

Test Results
Failed 0
Passed 102
Skipped 6
Total 108

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Contributor

@stuartnelson3 stuartnelson3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good! made some comments but i think they're mostly cosmetic

@trentm
Copy link
Member

trentm commented Mar 3, 2022

FYI: When testing with a build of the extension using this branch and with the Node.js APM agent changes to send the ?flushed=true signal (elastic/apm-agent-nodejs#2586), I no longer get the panics I reported in #133

@estolfo
Copy link
Contributor Author

estolfo commented Mar 3, 2022

Thanks for the update @trentm. This change should have indeed fixed that panic.
Also, @jlvoiseux has written a test in this PR that forces the scenario and confirms that the changes linked above resolve the panic.

@estolfo estolfo requested a review from stuartnelson3 March 4, 2022 12:04
Copy link
Contributor

@stuartnelson3 stuartnelson3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@estolfo estolfo force-pushed the estolfo/codereview-comments branch from 99501f3 to efb4054 Compare March 9, 2022 10:52
@estolfo estolfo merged commit 50c1105 into main Mar 9, 2022
@estolfo estolfo deleted the estolfo/codereview-comments branch March 9, 2022 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aws-λ-extension AWS Lambda Extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants