Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use connection reuse environment variable in Lambda #130

Merged
merged 3 commits into from
Jul 2, 2020

Conversation

lumishrestha
Copy link
Contributor

Replaces keepAlive property of https agent

@lumishrestha lumishrestha requested a review from a team as a code owner July 2, 2020 01:54
@changeset-bot
Copy link

changeset-bot bot commented Jul 2, 2020

🦋 Changeset is good to go

Latest commit: 5ce871a

We got this.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -3,7 +3,6 @@ import { Agent } from 'https';
import { SNS } from 'aws-sdk';

const agent = new Agent({
Copy link
Member

Choose a reason for hiding this comment

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

I think we can get rid of the custom agent entirely 🙂

@@ -59,6 +59,7 @@ functions:
reservedConcurrency: 20
timeout: 30
environment:
AWS_NODEJS_CONNECTION_REUSE_ENABLED: 1
Copy link

@tobyhei tobyhei Jul 2, 2020

Choose a reason for hiding this comment

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

edit: nvm

Copy link
Member

Choose a reason for hiding this comment

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

Ooh awesome! It would be nice to include this and also have links to the relevant doco for both environment variables, as part of documenting more of the rationale behind these templates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tobyhei any reasoning behind not adding that?

Copy link

Choose a reason for hiding this comment

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

Oh sorry, I was just going to suggest we add AWS_STS_REGIONAL_ENDPOINTS=regional but then thought it was scope creep for this PR so removed the comment

@lumishrestha lumishrestha merged commit c0b8f1c into master Jul 2, 2020
@lumishrestha lumishrestha deleted the update-serverless-template branch July 2, 2020 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants