-
Notifications
You must be signed in to change notification settings - Fork 224
Add http-1x codegen support for server SDK generation #4376
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: feature/http-1.x
Are you sure you want to change the base?
Conversation
| val httpDeps = HttpDependencies.create(settings.codegenConfig.http1x, rustSymbolProviderConfig.runtimeConfig) | ||
| EventStreamSymbolProvider(rustSymbolProviderConfig.runtimeConfig, it, CodegenTarget.SERVER, httpDeps.smithyHttp) |
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 recommend that we actually make RuntimeConfig "do the right thing". Or make it so that you MUST pass an http version setting when getting the dependencies that depend on it.
You could incorporate the http1x setting into RuntimeConfig potentially, then make things like smithyHttp http version aware?
I'm worried that the way the code works right now will be somewhat confusing for folks to deal with and create these really confusing compilation errors.
| ) | ||
| } | ||
| if (hasStreamingOperations(model, codegenContext.serviceShape)) { | ||
| println("[ServerRequiredCustomizations.pubUseSmithyPrimitivesHttp0x] Adding rt-tokio with http-body-0-4-x") |
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.
did you want to leave these printlns?
| class StatusCodeSensitivity(private val sensitive: Boolean, private val smithyHttpServer: RuntimeType) { | ||
| private val codegenScope = | ||
| arrayOf( | ||
| "SmithyHttpServer" to ServerCargoDependency.smithyHttpServer(runtimeConfig).toType(), |
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 think we should probably have ServerCargoDependency.smithyHttpServerAuto that automatically gives you the right one. I think the http version would actually fit pretty naturally into RuntimeConfig as an optional parameter.
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.
But if you just want to ship things like it is, I get that. I'm just kinda worried about the long term fallout of having it be non-centralized especially as we eventually try to get rid of the old codegen settings.
Merge the latest main to the `feature/http-1.x` branch, plus replace http-0.x constructs with http-1.x constructs as described in [Tips for merging to feature/http-1.x](#4384) - CI: ignore server related failures and semver hazard failure (the type being complained about `Uri` isn't exposed since its enclosing module `request` isn't exposed as pub, which confuses the semver hazard check) ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: Landon James <[email protected]> Co-authored-by: Russell Cohen <[email protected]> Co-authored-by: AWS SDK Rust Bot <[email protected]> Co-authored-by: AWS SDK Rust Bot <[email protected]> Co-authored-by: vcjana <[email protected]> Co-authored-by: Jason Gin <[email protected]> Co-authored-by: Aaron Todd <[email protected]>
## Motivation and Context Merge the main branch to the `feature/http-1.x` branch. It takes all from the main branch except for [this](#4392 (comment)), which is replaced by `http-1.x` construct. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
…est would be able to catch it
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
5c29232 to
9bb8f0d
Compare
This file was accidentally modified in this branch. Reverting to the upstream version to avoid merge conflicts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This file was accidentally modified in this branch. Reverting to the upstream version to avoid merge conflicts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This commit introduces comprehensive http-1x support to the server code generator,
enabling generation of server SDKs that work with either [email protected] or [email protected]
based on the
http-1xcodegen flag.Key Changes
New Infrastructure
HttpDependencies.kt: New data class that ensures cohesive HTTP dependency
selection, preventing accidental mixing of HTTP 0.x and 1.x versions. Provides
helper methods for accessing HTTP types (Request, Response, HeaderMap, etc.)
MultiVersionTestFailure.kt: Test infrastructure for tracking failures by
HTTP version, enabling tests to run against both http@0 and http@1
ServerCodegenConfig Updates
http1xboolean field to control HTTP version selection (config key:http-1x)false([email protected])ServerCodegenContext Updates
isHttp1()method to check if http-1x is enabledhttpDependencies()method to provide version-appropriate HTTP dependenciesthroughout code generation
ServerRequiredCustomizations
Protocol & Test Generator Updates
HTTP versions, with configurable test runtime mode (AS_CONFIGURED, BOTH, etc.)
references instead of hardcoded dependencies
Generator Updates
Updated generators to use HttpDependencies for type resolution:
Test Infrastructure
ability to run tests against both HTTP versions
based on http-1x flag
Core Changes
type generation
Testing
All protocol tests can now run against both HTTP versions to ensure compatibility.
Tests track failures by HTTP version for easier debugging.
Relates to #3362