-
Notifications
You must be signed in to change notification settings - Fork 221
[HTTP 1.x] Add legacy crates to maintain [email protected] support during migration #4372
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
base: main
Are you sure you want to change the base?
Conversation
|
can you target mainline with this instead? |
| event-stream = ["aws-smithy-eventstream"] | ||
| rt-tokio = ["aws-smithy-types/rt-tokio"] |
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.
hmmm...I guess we still need these features? event streams currently use aws-smithy-http based types. I guess the client won't use the legacy crate right? We should have a test case that ensures generated clients never include clients.
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.
The generated clients would never use these aws-smithy-legacy-* crates because the Kotlin class that adds these dependencies is defined in the HttpDependencies data class within the server module. There are no CargoDependencies defined for the legacy crates in the core module either. The client code has no reference to them unless someone explicitly adds them at a later stage.
I will add a test in the client to ensure they are not added.
| bytes-utils = "0.1" | ||
| # TODO(hyper1) - Complete the breaking changes by updating to http 1.x ecosystem fully in this crate. Also remove hyper 0.14 from dev | ||
| http-02x = { package = "http", version = "0.2.9" } | ||
| http-1x = { package = "http", version = "1" } |
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 don't need a lot of this stuff right? I guess I wonder if we should prune out the parts of aws-smithy-http-legacy we don't need. maybe its OK.
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 would like to keep it as it originally existed to ensure we don't break compatibility for users who still want to generate [email protected] SDKs. Therefore, these two crates are an exact copy of their original state before we decided to remove [email protected] support from them.
85912d3 to
5bd707e
Compare
5bd707e to
d25a40a
Compare
It now targets main. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
This PR introduces two new legacy crates (
aws-smithy-legacy-httpandaws-smithy-legacy-http-server) that maintain support for[email protected]while the codebase transitions to
[email protected].Background
As part of the HTTP 1.x migration effort (see branch
feature/http-1.x), we're adding support for generating server SDKs that depend on[email protected]via a newhttp-1xcodegen flag. However, we need to maintain backward compatibility with[email protected]for existing users.Why Legacy Crates?
Initially, we attempted to pin dependencies to the last versions that supported
[email protected]. However, this approach failed because:Solution
Creating dedicated legacy crates solves this problem by:
Changes in this PR
aws-smithy-legacy-httpcrate (forked fromaws-smithy-httpwith [email protected] support)aws-smithy-legacy-http-servercrate (forked fromaws-smithy-http-serverwith [email protected] support)rust-runtime/Cargo.tomlto include both legacy crates in the workspaceNote
This PR is split out from the main
[email protected]work to make it easier for reviewers to evaluate just the legacy crate additions separately from the codegen changes. By default,[email protected]remains the default - thehttp-1xflag must be explicitly enabled to use[email protected].