-
Notifications
You must be signed in to change notification settings - Fork 55
Enable general client generation from smithy models #275
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
Conversation
…g service clients.
ianbotsf
left a comment
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.
Correctness: This will likely break publishing to Maven because the new codegen artifacts go to software.amazon.smithy.kotlin instead of aws.sdk.kotlin like everything else in the project. You will probably have to gate the publishing of artifacts by group name similar smithy-kotlin#458.
|
Correctness: Apparently my list of test instructions were non-exhaustive. I checked this out locally and still ran into difficulty publishing to Nexus. I think the following changes are necessary in the root build.gradle.kts:
See smithy-kotlin/build.gradle.kts for an example. |
|
Updated based on your directive @ianbotsf . Ran the tests again and the same files were produced in the |
ianbotsf
left a comment
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.
Looks good to me!
docs/generate-smithy-client.md
Outdated
| ``` | ||
| 5. Disable internal annotation guards: | ||
| ```kotlin | ||
| kotlin.sourceSets.all { |
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.
this can be handled in the smithy-build.json
...
"kotlin-codegen": {
...
"build": {
"optInAnnotations": [
"aws.smithy.kotlin.runtime.util.InternalApi",
"aws.sdk.kotlin.runtime.InternalSdkApi"
]
}
}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.
fixed, thanks
docs/generate-smithy-client.md
Outdated
| └── smithy-build.json | ||
| ``` | ||
|
|
||
| ### 2. Updating the Generated Project to Build the Client |
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.
if you specify the build plugin setting to generate a "root" project you shouldn't need to update the generated build file:
"build": {
"rootProject": true
}This is how the protocol tests work, we generate a "full" independent gradle project with a settings.gradle.kts, etc. If you don't generate a full project we assume that the generated code is going to be a subproject and configured by a parent somewhere in the build.
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.
fixed, thanks
Create code sample project with weather example Update docs and dependencies
| maven { | ||
| name = "kotlinSdkLocal" | ||
| url = uri(TODO("set your local repository path")) | ||
| // e.g. | ||
| //url = uri("file:///tmp/aws-sdk-kotlin-repo/m2") | ||
| } |
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.
Question: Do we want to leave mavenLocal() here for local building/testing using the example projects?
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.
Hmm, my assumption was that once we were public we wouldn't need this. What in particular will need a local dependency?
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.
discussed offline and added mavenLocal
…
Issue # N/A
Description of changes
Additional Testing
Ian provided some steps I performed locally to verify expected behavior:
I needed to add the crt repo to my composite build in order to keep the crt artifacts from polluting the m2 repo during build. Once that was done, the results:
Building with group name
aws.sdk.kotlinafter deleting~/.m2Building with group name
software.amazon.smithy.kotlinafter deleting~/.m2I did not verify every pop contained the expected group, but spot checked and didn't find any surprises.
Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.