-
Notifications
You must be signed in to change notification settings - Fork 407
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
feat(feature_flags): allow customers to bring their own boto3 client and session #4717
feat(feature_flags): allow customers to bring their own boto3 client and session #4717
Conversation
Signed-off-by: Adrián Tomás <[email protected]>
Hello @adriantomas! What a great discovery to improve this utility! Do you mind creating an issue to link here? And I think maybe there are some tests left, right? |
Sure, let me take a look ;) |
sdk_client
argument to AppConfigStore __init__
Signed-off-by: Adrián Tomás <[email protected]>
Signed-off-by: Adrián Tomás <[email protected]>
Signed-off-by: Adrián Tomás <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4717 +/- ##
========================================
Coverage 96.42% 96.43%
========================================
Files 223 223
Lines 10756 10757 +1
Branches 2001 2001
========================================
+ Hits 10372 10373 +1
Misses 270 270
Partials 114 114 ☔ View full report in Codecov by Sentry. |
Hello @adriantomas! I've made a few changes to this PR to adjust some minor things. Here's a summary of what I've done: 1 - I've changed the name from Please let me know your thoughts. We want to include this change in our tomorrow's release. Thanks a lot for working on this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for this PR @adriantomas! APPROVED!! 🚀
Issue number: #4728
Summary
This pull request adds an argument to be able to send a boto3 client from AppConfigData to the
AppConfigStore
initializer.Changes
AppConfigStore
.User experience
Enables the following:
AppConfigData
boto3 client for other uses not related to theAppConfigStore
and allow to comply with a best practice.endpoint_url
argument of the client.Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.