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

add: RFC for Supporting SelectObjectContent's unique ScanRange behavior #2022

Closed

Conversation

Velfi
Copy link
Contributor

@Velfi Velfi commented Nov 22, 2022

Motivation and Context

aws-smithy-sdk#630


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Velfi Velfi requested review from a team as code owners November 22, 2022 16:51
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@Velfi Velfi changed the title add: RFC for Supporting SelectObjectContent's unique ScanRange behavior add: RFC for Supporting SelectObjectContent's unique ScanRange behavior Nov 22, 2022
Copy link
Collaborator

@jdisanti jdisanti left a comment

Choose a reason for hiding this comment

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

Nicely written RFC! Seems to me like we should just implement IDLv2 default support, and then maybe we can consider adding a hook for custom methods/fields and serialization hooks on code generated structs to handle the "last n-bytes" case later.

Why this RFC should be rejected
-------------------------------

There are several reasons to reject this RFC:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another reason: In the unlikely event that S3 modifies ScanRange, the handwritten implementation will be out of sync.

Comment on lines +198 to +199
We have already committed to supporting [IDLv2] and implementing that would allow users to set `start=0` or unset it.
Although IDLv2 support would NOT support the custom `end` value behavior, we'd be halfway there. Any future S3
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a little confusing because it seemed like the handwritten ScanRange implementation was to be applied to the IDLv2 implementation. But this sentence makes it seem like the ScanRange proposal is actually for an IDLv1 implementation?

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@jdisanti jdisanti added rfc Marks a PR as an RFC client labels Dec 20, 2022
@Velfi
Copy link
Contributor Author

Velfi commented Mar 22, 2023

I'm closing this since we've decided to handle this issue by implementing IDLv2 support instead.

@Velfi Velfi closed this Mar 22, 2023
@landonxjames landonxjames deleted the rfc/supporting-SelectObjectContent-unique-ScanRange-behavior branch January 13, 2025 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client rfc Marks a PR as an RFC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants