Skip to content

Connection builder simplification #276

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

Merged
merged 5 commits into from
Mar 16, 2022

Conversation

TwistedTwigleg
Copy link
Contributor

Description of changes:

Adjusted samples to use the changes in this PR: awslabs/aws-crt-python#334

This makes the code much smaller and removes the requirement to create and manage a HostResolver, EventLoopGroup, and ClientBootstrap to create a connection.

Requires changes the changes in the PR to function.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Now it is no longer required to make and manage a HostResolver,
EventLoopGroup, or ClientBootstrap. Also added singleton versions
of all three classes and adjusted constructors to make passing
a ClientBootstrap optional in many cases.

Requires changes to the Python crt repository to function correctly.
@TwistedTwigleg TwistedTwigleg changed the title First pass at builder simplification for Python V2 SDK Connection builder simplification Feb 15, 2022
Copy link
Contributor

@xiazhvera xiazhvera left a comment

Choose a reason for hiding this comment

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

The PR seems good to me. Just need to wait for release of awslabs/aws-crt-python#334.

Copy link
Contributor

@xiazhvera xiazhvera left a comment

Choose a reason for hiding this comment

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

I got compile error with using_static_defaults. I think it was deprecated, is it? And the current main has updated crt to 0.13.5, we might want to do that as well.

@TwistedTwigleg
Copy link
Contributor Author

Yeah, I think it is deprecated. I'll update this PR and the samples to use the latest changes (and make sure it compiles) first thing tomorrow morning 👍

@TwistedTwigleg
Copy link
Contributor Author

Thanks for the review! I fixed the issue with using_static_defaults, merged the latest version of master, and now all the tests pass 👍

@TwistedTwigleg
Copy link
Contributor Author

Thanks for the review! Merging into main...

@TwistedTwigleg TwistedTwigleg merged commit 9968fd2 into main Mar 16, 2022
@TwistedTwigleg TwistedTwigleg deleted the ConnectionBuilderSimplification branch March 16, 2022 17:45
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.

2 participants