smithy-lang / smithy-rs

Code generation for the AWS SDK for Rust, as well as server and generic smithy client generation.
Apache License 2.0
496 stars 187 forks source link

codegen-server support for restXml (S3) #903

Open guymguym opened 2 years ago

guymguym commented 2 years ago

Hi

I was trying to run codegen-server-test for S3 but it fails with the following message:

No matching protocol — service offers: [aws.protocols#restXml]. We offer: [aws.protocols#restJson1]

Is there a plan to support server codegen for restXml? I am new to the project so I might have used it incorrectly, so I am pasting my diffs and commands below. TIA

Here is the diffs over v0.30.0-alpha:

$ git status
HEAD detached at v0.30.0-alpha
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   aws/rust-runtime/aws-http/src/lib.rs
        modified:   aws/rust-runtime/aws-types/src/lib.rs
        modified:   codegen-server-test/build.gradle.kts

no changes added to commit (use "git add" and/or "git commit -a")

$ git diff --minimal 
diff --git a/aws/rust-runtime/aws-http/src/lib.rs b/aws/rust-runtime/aws-http/src/lib.rs
index 68e8a029..6952e54e 100644
--- a/aws/rust-runtime/aws-http/src/lib.rs
+++ b/aws/rust-runtime/aws-http/src/lib.rs
@@ -7,7 +7,7 @@

 #![warn(
     missing_docs,
-    missing_crate_level_docs,
+    rustdoc::missing_crate_level_docs,
     missing_debug_implementations,
     rust_2018_idioms,
     unreachable_pub
diff --git a/aws/rust-runtime/aws-types/src/lib.rs b/aws/rust-runtime/aws-types/src/lib.rs
index 3421b5f2..437f81c9 100644
--- a/aws/rust-runtime/aws-types/src/lib.rs
+++ b/aws/rust-runtime/aws-types/src/lib.rs
@@ -7,7 +7,7 @@

 #![warn(
     missing_docs,
-    missing_crate_level_docs,
+    rustdoc::missing_crate_level_docs,
     missing_debug_implementations,
     rust_2018_idioms,
     unreachable_pub
diff --git a/codegen-server-test/build.gradle.kts b/codegen-server-test/build.gradle.kts
index fb849239..8c293ad8 100644
--- a/codegen-server-test/build.gradle.kts
+++ b/codegen-server-test/build.gradle.kts
@@ -23,8 +23,9 @@ dependencies {
 data class CodegenTest(val service: String, val module: String, val extraConfig: String? = null)

 val CodegenTests = listOf(
-    CodegenTest("com.amazonaws.simple#SimpleService", "simple"),
-    CodegenTest("com.amazonaws.ebs#Ebs", "ebs")
+    CodegenTest("com.amazonaws.s3#AmazonS3", "s3")
+    //CodegenTest("com.amazonaws.simple#SimpleService", "simple"),
+    //CodegenTest("com.amazonaws.ebs#Ebs", "ebs")
 )

This is the output of running gradle:

$ ./gradlew :codegen-server-test:test

> Configure project :codegen-server-test
(scanned and found a Smithy CLI version 1.14.1. You will need to add an explicit dependency on smithy-model if publishing a JAR)

> Configure project :codegen-test
(scanned and found a Smithy CLI version 1.14.1. You will need to add an explicit dependency on smithy-model if publishing a JAR)

> Configure project :aws:sdk
null
Discovered protocol tests for batch.json
Discovered protocol tests for ebs.json
Discovered protocol tests for glacier.json
Discovered protocol tests for s3.json
Discovered protocol tests for sqs.json
(scanned and found a Smithy CLI version 1.13.0. You will need to add an explicit dependency on smithy-model if publishing a JAR)

> Configure project :aws:sdk-codegen-test
(scanned and found a Smithy CLI version 1.14.1. You will need to add an explicit dependency on smithy-model if publishing a JAR)
Building Smithy model sources: [/code/smithy-rs/codegen-server-test/model]
Loading Smithy configs: [/code/smithy-rs/codegen-server-test/smithy-build.json]
Validation result: 0 ERROR(s), 0 DANGER(s), 0 WARNING(s), 0 NOTE(s)
Validated 2183 shapes in model
Completed projection source (2183 shapes): /code/smithy-rs/codegen-server-test/build/smithyprojections/codegen-server-test/source

Projection s3 failed: software.amazon.smithy.codegen.core.CodegenException: No matching protocol — service offers: [aws.protocols#restXml]. We offer: [aws.protocols#restJson1]
software.amazon.smithy.rust.codegen.server.smithy.protocols.ServerProtocolLoader.protocolFor(ServerProtocolLoader.kt:31)
software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenVisitor.<init>(ServerCodegenVisitor.kt:81)
software.amazon.smithy.rust.codegen.server.smithy.RustCodegenServerPlugin.execute(RustCodegenServerPlugin.kt:46)
software.amazon.smithy.build.SmithyBuildImpl.applyPlugin(SmithyBuildImpl.java:387)
software.amazon.smithy.build.SmithyBuildImpl.applyProjection(SmithyBuildImpl.java:319)
software.amazon.smithy.build.SmithyBuildImpl.executeSerialProjection(SmithyBuildImpl.java:222)
software.amazon.smithy.build.SmithyBuildImpl.lambda$applyAllProjections$5(SmithyBuildImpl.java:187)
java.base/java.util.concurrent.ForkJoinTask$AdaptedCallable.exec(ForkJoinTask.java:1453)
java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1016)
java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1665)
java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1598)
java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

Smithy built 1 projection(s), 3 plugin(s), and 5 artifacts
The following 1 Smithy build projection(s) failed: [s3]

> Task :codegen-server-test:smithyBuildJar FAILED
Running smithy build
The following 1 Smithy build projection(s) failed: [s3]

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':codegen-server-test:smithyBuildJar'.
> The following 1 Smithy build projection(s) failed: [s3]

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

Deprecated Gradle features were used in this build, making it incompatible with Gradle 7.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/6.6.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD FAILED in 4s
13 actionable tasks: 2 executed, 11 up-to-date
crisidev commented 2 years ago

Hello, thanks for submitting the issue! Currently the codegen-server supports only a subset of restJson1 protocol. We are working hard to first finish the support for this protocol and for the runtime needed to spawn and handle a full service.

We have plans to support all the wire protocols offered by Smithy, but unfortunately I don't have a timeline for when restXml will be added.

crisidev commented 2 years ago

To give a little bit more context in what is needed to support restXml:

1) Implement the protocol specification in server-codegen: this is done-ish in https://github.com/awslabs/smithy-rs/commit/6184024648bfd5119da7ecf9e471e6f0b1f89d43 2) Implement server specific serializer methods: https://github.com/awslabs/smithy-rs/blob/main/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/serialize/XmlBindingTraitSerializerGenerator.kt#L192-L199 3) Implement server specific deserializer methods: https://github.com/awslabs/smithy-rs/blob/main/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/protocols/parse/XmlBindingTraitParserGenerator.kt#L246-L248 4) Add the restXml ContentType parsing support in the runtime: https://github.com/awslabs/smithy-rs/blob/main/rust-runtime/aws-smithy-http-server/src/protocols.rs#L11 5) Use the right ContentType parsing in ServerHttpProtocolGenerator: https://github.com/awslabs/smithy-rs/blob/main/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpProtocolGenerator.kt#L493

guymguym commented 2 years ago

Hey @crisidev Would it be welcome if I attempt to make a PR on this? BTW I wonder if the input/output/error types are shared with the client codegen? or are completely separate? I was looking for a way to convert a server request to a client request too and back...

crisidev commented 2 years ago

PRs are of course super welcome! I will check and comment on your work tomorrow! Thanks a million for your contribution

rcoh commented 2 years ago

re: input / output shapes: They're completely separate because the backwards compatibility guarantees around server types are actually different from those on the client types (you can have old clients, but not old servers, generally).

Generating converters, however, would be an interesting thing to add (and easy to code generate)

drganjoo commented 5 months ago

This is a low priority item as restXml is not a modern protocol. Are you still interested in using this protocol?

drganjoo commented 4 months ago

It looks like nobody is interested in supporting restXml in the server. We are leaning towards removing the existing initial support.

rcoh commented 4 months ago

yeah it seems like supporting S3 would be the main reason (but it seems like that mostly uses custom code anyway)