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

RPCv2 and cbor support #3006

Open
wants to merge 45 commits into
base: version-3
Choose a base branch
from
Open

RPCv2 and cbor support #3006

wants to merge 45 commits into from

Conversation

mullermp
Copy link
Contributor

WIP for implementing cbor and smithy rpcv2

Encoder and decoder created by @alextwoods


module Aws
module Cbor
Tagged = Struct.new(:tag, :value) do
Copy link
Contributor

Choose a reason for hiding this comment

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

the cbor gem also defines its own Tagged class: https://github.com/cabo/cbor-ruby/blob/master/doclib/cbor/tagged.rb#L6
Which it will return when decoding items.

I think we have two options on how to handle this:

  1. In our cbor gem engine adaptor, we replace any Cbor Tagged objects with Aws::Taged objects.
  2. We ensure the intrefaces are the same (ie, remove the use of Struct and just define a class with attr accessors) for our Tagged object. Then just rely no ruby's duck typing.

@mullermp mullermp marked this pull request as ready for review April 16, 2024 21:07
@aws aws deleted a comment from github-actions bot Apr 21, 2024
@aws aws deleted a comment from github-actions bot Apr 21, 2024
@aws aws deleted a comment from github-actions bot Apr 21, 2024
@aws aws deleted a comment from github-actions bot Apr 21, 2024
@aws aws deleted a comment from github-actions bot Apr 21, 2024
@aws aws deleted a comment from github-actions bot Apr 21, 2024
@aws aws deleted a comment from github-actions bot Apr 21, 2024
@aws aws deleted a comment from github-actions bot Apr 29, 2024
@aws aws deleted a comment from github-actions bot Apr 29, 2024
Copy link
Contributor

@alextwoods alextwoods left a comment

Choose a reason for hiding this comment

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

Nice - updates look good

'union' => false, # should remain false
'streaming' => true,
'requiresLength' => true,
'union' => false, # not ignored - should remain false
Copy link
Contributor

Choose a reason for hiding this comment

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

"not ignored" is slightly confusing. Maybe "handled separately"

Object.send(:remove_const, :ProtocolsJsonQuery)
end

it 'selects query if is the only choice' do
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you meant to say, selects query if it is the only choice here like line 20

end

def end_indefinite_collection
# write the stop sequenece
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: sequence

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants