-
Notifications
You must be signed in to change notification settings - Fork 24
Release: smithy_http-0.2.0, smithy_aws_core-0.1.1 #580
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
Conversation
nateprewitt
left a comment
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.
Few high-level questions/comments, otherwise looks good.
| public static final PythonDependency AWS_CRT = new PythonDependency( | ||
| "awscrt", | ||
| ">=0.23.10", | ||
| "~=0.28.2", |
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.
We need to be careful trying to strictly pin this. We did it in boto3 because it was an optional opt in. Now that the CRT may be a required dependency we should be trying to broaden the range while still making sure it works.
Is this the earliest version that supports the new APIs?
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.
Yea, we depend on the changes required in awscrt v0.28.2. I mentioned that here which is where we actually define this dependency. This change here is actually not used anywhere. I'm just updating here since we may use this code in the future (if we decide to make CRT a required dependency for all clients). We can also consider removing it until we need it again.
| "changes": [ | ||
| { | ||
| "type": "dependency", | ||
| "description": "**Updated**: `smithy-http` from `~=0.1.0` to `~=0.2.0`." |
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.
Just curious is this the syntax we're planning to use going forward? It seems like it may be redundant to label every update with updated.
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.
I added **Updated** since there can also be **Removed** and **Added**. However, **Updated** will be the most common.
I'll update this to:
Bump `smithy-http` from `~=0.1.0` to `~=0.2.0`
Let me know if this addresses your concern or if I misunderstood you.
| "changes": [ | ||
| { | ||
| "type": "breaking", | ||
| "description": "Update `AWSCRTHTTPClient` to integrate with the new AWS CRT async interfaces. ([#573](https://github.com/smithy-lang/smithy-python/pull/573))" |
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.
This may warrant a tiny bit more info to better understand why it's breaking.
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.
Addressed in c41c78b. Let me know what you think
| { | ||
| "changes": [ | ||
| { | ||
| "type": "breaking", |
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.
Also for my understanding, did we canonicalize "breaking" as an official type going forward?
Is this for pre-1.0 or is the intent to use this after that?
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.
Also for my understanding, did we canonicalize "breaking" as an official type going forward?
Yup, the allow list is here: https://github.com/smithy-lang/smithy-python/blob/develop/scripts/changelog/new-entry.py#L58. Boto3 uses feature for all breaking changes which is odd. I didn't want to keep that pattern so I added a breaking option here. We do have validation to prevent typos of unsupported types.
Is this for pre-1.0 or is the intent to use this after that?
Yea, I have a TODO to remove the breaking option after v1.0.0.
* smithy-http v0.2.0 * smithy-aws-core v0.1.1 * Update code-generator to use latest package versions
This PR adds changelog entries, bumps versions, and updates dependencies for the following packages:
smithy-http-0.1.0-->0.2.0smithy-aws-core-0.1.0-->0.1.1smithy-httpis bumped to~=0.2.0By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.