Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Problems with rendering of httpResponseCode binding #1229

Closed
david-perez opened this issue Mar 2, 2022 · 0 comments · Fixed by #1405
Closed

Problems with rendering of httpResponseCode binding #1229

david-perez opened this issue Mar 2, 2022 · 0 comments · Fixed by #1405
Assignees
@82marbag
Labels
server Rust server SDK

Comments

@david-perez
Copy link
Contributor

There are two problems with the code that handles this trait:

  1. We're not falling back to the http trait if the targetted member is optional and no value is provided by the service implementer. See Make httpResponseCode be coupled with required smithy#1098 (comment).
  2. It only works when the targetted member is optional.
@david-perez david-perez added the server Rust server SDK label Mar 2, 2022
@82marbag 82marbag self-assigned this May 10, 2022
82marbag pushed a commit that referenced this issue May 24, 2022
Correctly generate http response codes
fac867c 
http response codes will be:
* @http's code, if @httpResponseCode is not set
* the default one, if neither of the above are set

@httpResponseCode honors @required and generates code that
doesn't use Option when applied.

Closes: #1229

Signed-off-by: Daniele Ahmed <[email protected]>
@82marbag 82marbag mentioned this issue May 24, 2022
Merged
2 tasks
82marbag pushed a commit that referenced this issue May 24, 2022
Correctly generate http response codes
5aafb41 
http response codes will be:
* @http's code, if @httpResponseCode is not set
* the default one, if neither of the above are set

@httpResponseCode honors @required and generates code that
doesn't use Option when applied.

Closes: #1229

Signed-off-by: Daniele Ahmed <[email protected]>
82marbag pushed a commit to 82marbag/smithy that referenced this issue May 26, 2022
Add tests for httpResponseCode
244009d 
These tests ensure that
* @required and @httpResponseCode are correctly handled
* @http's code is used in place of @httpResponseCode when the latter is
  not used

They were not handled correctly in
smithy-lang/smithy-rs#1229.

Signed-off-by: Daniele Ahmed <[email protected]>
@82marbag 82marbag mentioned this issue May 26, 2022
Merged
82marbag added a commit that referenced this issue May 27, 2022
@82marbag
Correctly generate http response codes (#1405)
1f31eae 
* Correctly generate http response codes

http response codes will be:
* `@httpResponseCode` if set
* `@http`'s code, if `@httpResponseCode` is not set
* the default one, if neither of the above are set

@httpResponseCode honors @required and generates code that
doesn't use Option when applied.

Closes: #1229
gosar pushed a commit to smithy-lang/smithy that referenced this issue Jun 15, 2022
@82marbag
Add tests for httpResponseCode (#1241)
359fc17 
Add tests for httpResponseCode

These tests ensure that
* @required and @httpResponseCode are correctly handled
* @http's code is used in place of @httpResponseCode when the latter is
  not used

They were not handled correctly in
smithy-lang/smithy-rs#1229.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@82marbag 82marbag

Labels
server Rust server SDK
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Correctly generate http response codes smithy-lang/smithy-rs
2 participants
@david-perez @82marbag