fix: Move boto3 client creation to a computed property #1059
Conversation
|
|
||
| @property | ||
| def lambda_client(self): | ||
| return self._lambda_client or boto3.client('lambda') |
There was a problem hiding this comment.
I'm ok with the change, just curious if it will change our failure mode scenarios anywhere.
There was a problem hiding this comment.
No. This just means we create the default boto3 client when the property is accessed instead of on creation. This will mean we will be creating a new boto3 client on every access though. Maybe this is a even better way?
self._lambda_client = self._lambda_client or boto3.client('lambda')
return self._lambda_client
jfuss
force-pushed
the
computed-property-for-boto3
branch
from
ca14777
to
7cac013
Compare
jfuss
force-pushed
the
computed-property-for-boto3
branch
from
7cac013
to
73c20a6
Compare
|
Finally had some time to dig into the build failures. Turns out, boto3 sessions are not thread safe. |
| LOG.debug("Loading AWS credentials from session with profile '%s'", self.aws_profile) | ||
| # boto3.session.Session is not thread safe. To ensure we do not run into a race condition with start-lambda | ||
| # or start-api, we create the session object here on every invoke. | ||
| session = boto3.session.Session(profile_name=self.aws_profile, region_name=self.aws_region) |
There was a problem hiding this comment.
Should we explicitly call out new session in debug statement as well?
There was a problem hiding this comment.
For the customer, it really doesn't matter. I don't think a debug statement for this makes sense.
For further context, this is how we did it prior to the work we did on Layers. We got fancy in using the default session. Turns out that only really worked because we were calling boto3.client. Once we moved this call to be computed, I was seeing sporadic failures on a KeyError within botocore. So I reverted us back to our previous state, which ensures Sessions get created per invoke. Which means for start-lambda and start-api that could be per request.
Issue #, if available:
#885
Description of changes:
Checklist:
make prpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.