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

Add Smithy RPC v2 trait definition and implementation #1583

Merged
merged 7 commits into from
Feb 9, 2023
Merged

Add Smithy RPC v2 trait definition and implementation #1583

merged 7 commits into from
Feb 9, 2023

Conversation

crisidev
Copy link
Contributor

Description of changes:

This is the initial trait implementation for a new protocol, Smithy RPC v2.

More PRs will come, to add documentation and protocol tests. This will be merged against the smithy-rpc-v2 branch to prevent polluting main until this effort is concluded.

@kstich for visibility.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@crisidev
Add Smithy RPC v2 trait definition and implementation
28dff25 
Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>
@crisidev crisidev requested a review from a team as a code owner January 18, 2023 18:20
kstich
kstich requested changes Feb 1, 2023
View reviewed changes
smithy-protocols-traits/src/main/resources/META-INF/smithy/smithy.protocols.rpcv2.smithy Outdated Show resolved Hide resolved
smithy-protocols-traits/src/main/resources/META-INF/smithy/smithy.protocols.rpcv2.smithy Outdated Show resolved Hide resolved
smithy-protocols-traits/src/main/resources/META-INF/smithy/smithy.protocols.rpcv2.smithy Show resolved Hide resolved
smithy-protocols-traits/src/main/resources/META-INF/smithy/smithy.protocols.rpcv2.smithy Outdated Show resolved Hide resolved
smithy-protocols-traits/src/main/resources/META-INF/smithy/smithy.protocols.rpcv2.smithy Outdated Show resolved Hide resolved
smithy-protocols-traits/src/main/java/software/amazon/smithy/protocols/traits/package-info.java Outdated Show resolved Hide resolved
...tocols-traits/src/main/java/software/amazon/smithy/protocols/traits/SmithyProtocolTrait.java Outdated Show resolved Hide resolved
...protocols-traits/src/main/java/software/amazon/smithy/protocols/traits/SmithyRpcV2Trait.java Outdated
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.traits.AbstractTrait;

public final class SmithyRpcV2Trait extends SmithyProtocolTrait {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followup task: will need validation on at least one formats entry, the value of the format(s), and the http/eventStreamHttp values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have fixed everything else in the meantime. This task will require me to understand a little bit more of smithy before being able to implement it. Do you want this to be part of this PR or should we address it in the next one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I have implemented the validator :) Now it just matter of writing a couple of tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to remove the Validator for now, so that we can start by shipping the trait.

smithy-protocols-traits/build.gradle Outdated Show resolved Hide resolved
@crisidev crisidev requested a review from kstich February 8, 2023 15:44
@crisidev crisidev requested a review from kstich February 9, 2023 10:51
kstich
kstich reviewed Feb 9, 2023
View reviewed changes
smithy-protocols-traits/src/main/java/software/amazon/smithy/protocols/traits/Rpcv2Trait.java Outdated
private final List<String> eventStreamHttp;
private final List<String> format;

public static final ShapeId ID = ShapeId.from("smithy.protocols#Rpcv2");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be a lowercase rpcv2 to exactly match the name of the trait shape.

...raits/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.Validator Outdated
@@ -0,0 +1 @@
software.amazon.smithy.aws.traits.protocols.ProtocolHttpValidator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should remove this file as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I thought we could just piggy-back on this validator for http / httpStreaming. I'll remove it.

@kstich kstich merged commit b628fa5 into smithy-lang:smithy-rpc-v2 Feb 9, 2023
mtdowling
mtdowling reviewed Feb 9, 2023
View reviewed changes
smithy-protocols-traits/src/main/resources/META-INF/smithy/smithy.protocols.rpcv2.smithy
}

/// A list of String shapes.
list StringList {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should be @private

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix it!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1610

kstich pushed a commit that referenced this pull request Aug 1, 2023
@crisidev @kstich
Add Smithy RPC v2 trait (#1583)
40a1d8d 
* Add Smithy RPC v2 trait definition and implementation

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>

* Add support for wire formats

* Address PR comments

* Rename SmithyRpcV2* to just Rpcv2

* Finish refactor to just have Rpcv2

* Address PR comments. Remove the validator for now

* Address comments from PR

---------

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>
david-perez pushed a commit to david-perez/smithy that referenced this pull request Nov 16, 2023
@crisidev @david-perez
Add Smithy RPC v2 trait (smithy-lang#1583)
c8539ac 
* Add Smithy RPC v2 trait definition and implementation

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>

* Add support for wire formats

* Address PR comments

* Rename SmithyRpcV2* to just Rpcv2

* Finish refactor to just have Rpcv2

* Address PR comments. Remove the validator for now

* Address comments from PR

---------

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>
kstich pushed a commit that referenced this pull request Nov 20, 2023
@crisidev @kstich
Add Smithy RPC v2 trait (#1583)
650f5e0 
* Add Smithy RPC v2 trait definition and implementation

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>

* Add support for wire formats

* Address PR comments

* Rename SmithyRpcV2* to just Rpcv2

* Finish refactor to just have Rpcv2

* Address PR comments. Remove the validator for now

* Address comments from PR

---------

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>
kstich pushed a commit that referenced this pull request Jan 2, 2024
@crisidev @kstich
Add Smithy RPC v2 trait (#1583)
b64ceac 
* Add Smithy RPC v2 trait definition and implementation

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>

* Add support for wire formats

* Address PR comments

* Rename SmithyRpcV2* to just Rpcv2

* Finish refactor to just have Rpcv2

* Address PR comments. Remove the validator for now

* Address comments from PR

---------

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>
kstich pushed a commit that referenced this pull request Mar 11, 2024
@crisidev @kstich
Add Smithy RPC v2 trait (#1583)
a2279f8 
* Add Smithy RPC v2 trait definition and implementation

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>

* Add support for wire formats

* Address PR comments

* Rename SmithyRpcV2* to just Rpcv2

* Finish refactor to just have Rpcv2

* Address PR comments. Remove the validator for now

* Address comments from PR

---------

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>
kstich pushed a commit that referenced this pull request Mar 22, 2024
@crisidev @kstich
Add Smithy RPC v2 trait (#1583)
1ca1f8a 
* Add Smithy RPC v2 trait definition and implementation

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>

* Add support for wire formats

* Address PR comments

* Rename SmithyRpcV2* to just Rpcv2

* Finish refactor to just have Rpcv2

* Address PR comments. Remove the validator for now

* Address comments from PR

---------

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>
kstich pushed a commit that referenced this pull request Mar 27, 2024
@crisidev @kstich
Add Smithy RPC v2 trait (#1583)
e0be07f 
* Add Smithy RPC v2 trait definition and implementation

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>

* Add support for wire formats

* Address PR comments

* Rename SmithyRpcV2* to just Rpcv2

* Finish refactor to just have Rpcv2

* Address PR comments. Remove the validator for now

* Address comments from PR

---------

Signed-off-by: Bigo <1781140+crisidev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhernandez35 rhernandez35 rhernandez35 left review comments

@mtdowling mtdowling mtdowling left review comments

@kstich kstich kstich approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@crisidev @mtdowling @kstich @rhernandez35