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.

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

Model simple.smithy should emit a DANGER event #1489

Closed
crisidev opened this issue Jun 23, 2022 · 5 comments
Closed

Model simple.smithy should emit a DANGER event #1489

crisidev opened this issue Jun 23, 2022 · 5 comments
Labels
help wanted Extra attention is needed server Rust server SDK

Comments

@crisidev
Copy link
Contributor

The model simple.smithy should emit a DANGER event like

DANGER: com.amazonaws.simple#StoreServiceBlob (HttpMethodSemantics)
     @ codegen-server-test/python/model/simple.smithy
     |
 111 | @http(method: "GET", uri: "/service/{id}/blob")
     |       ^
     = This operation uses the `GET` method in the `http` trait, but the `content` member is sent as the payload of the request because it is marked with the `httpPayload` trait. Many HTTP clients do not support payloads with GET requests. Consider binding this member to other parts of the HTTP request such as a query string parameter using the `httpQuery` trait, a header using the `httpHeader` trait, or a path segment using the `httpLabel` trait.

caused by this line.

The DANGER event is not emitted and we don't even get a warning.

I have found a very weird way to make smithy emit the DANGER event:

The python subfolder of codegen-server-test contains a folder called model which is a symlink to the model folder of its parent codegen-server-test. Changing this symlink into a real folder and linking the models inside it triggers the DANGER event when running ./gradlew codegen-server-test:python:assemble.

I think you can imagine my surprise when this happened.

I am not sure if this is an issue with smithy-rs or smithy itself or a combination of the two.

Repro:

@crisidev crisidev added the help wanted Extra attention is needed label Jun 23, 2022
@rcoh
Copy link
Collaborator

rcoh commented Jun 23, 2022

I think this comes from smithy and not from smithy-rs

@Velfi Velfi added the server Rust server SDK label Jun 24, 2022
@82marbag
Copy link
Contributor

82marbag commented Sep 6, 2022

@jdisanti might have stumbled on this too

@jdisanti
Copy link
Collaborator

jdisanti commented Sep 7, 2022

When I run the codegen-test:check target, I see evidence that validation of the classpath imported models is actually occurring:

WARNING: - (ModelDeprecation)
     @ /Users/jdisanti/src/smithy-rs/codegen-test/model/misc.smithy
     |
   1 | $version: "1.0"
     |           ^
     = Smithy IDL 1.0 is deprecated. Please upgrade to 2.0.

However, no DANGER messages appear even though they should definitely be present in this misc.smithy at main@e3239e1a17ea0165849ce5379ae0fe40bdb16577.

I think this may be a Smithy bug.

@jdisanti
Copy link
Collaborator

jdisanti commented Sep 7, 2022

Filed smithy-lang/smithy#1387 on Smithy.

@crisidev
Copy link
Contributor Author

Fixed by: smithy-lang/smithy#1525

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed server Rust server SDK
Projects
None yet
Development

No branches or pull requests

5 participants