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

SelectObjectContent ScanRange does not accept zero-start #630

Closed
Licht-T opened this issue Oct 7, 2022 · 13 comments
Closed

SelectObjectContent ScanRange does not accept zero-start #630

Licht-T opened this issue Oct 7, 2022 · 13 comments
Labels
bug This issue is a bug. p2 This is a standard priority issue

Comments

@Licht-T
Copy link

Licht-T commented Oct 7, 2022

Describe the bug

ScanRange is the zero-start range, but Rust SDK doesn't accept zero as Start value.

if input.start != 0 {
let mut inner_writer = scope.start_el("Start").finish();
inner_writer.data(aws_smithy_types::primitive::Encoder::from(input.start).encode());
}
if input.end != 0 {
let mut inner_writer = scope.start_el("End").finish();
inner_writer.data(aws_smithy_types::primitive::Encoder::from(input.end).encode());
}

Expected Behavior

When the range is [0, 54], the ScanRange element of request body should be:

<ScanRange><Start>0</Start><End>54</End></ScanRange>

Current Behavior

When the range is [0, 54], it is currently:

<ScanRange><End>54</End></ScanRange>

Reproduction Steps

                    let mut select_object_content_req = client
                        .select_object_content()
                        .bucket(bucket.to_owned())
                        .key(key.to_owned())
(snip)
                        .scan_range(
                            ScanRange::builder()
                                .start(0)
                                .end(54)
                                .build(),
                        )

Possible Solution

No response

Additional Information/Context

No response

Version

v0.49.0

Environment details (OS name and version, etc.)

Ubuntu 22.04

Logs

No response

@Licht-T Licht-T added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 7, 2022
@Velfi Velfi removed the needs-triage This issue or PR still needs to be triaged. label Oct 7, 2022
@Velfi Velfi self-assigned this Oct 7, 2022
@Velfi
Copy link
Contributor

Velfi commented Oct 7, 2022

@Licht-T Thanks for reporting this. I'll take a look today and see if I can fix it.

@Velfi
Copy link
Contributor

Velfi commented Oct 7, 2022

I was able to reproduce this issue. It should be solvable by customizing the serializer to serialize zeros for that operation. Working on that now.

@Velfi
Copy link
Contributor

Velfi commented Oct 7, 2022

I was able to fix this with a serializer codegen update. I'll update this post again once the fix is released (should be less than 2 weeks.)

@Licht-T
Copy link
Author

Licht-T commented Oct 7, 2022

Thanks, @Velfi. Looking forward to the PR merged.

@Velfi
Copy link
Contributor

Velfi commented Oct 17, 2022

It looks like this is going to take longer than I expected. I'm going to be working this week to find a better solution to this issue.

@Velfi
Copy link
Contributor

Velfi commented Oct 26, 2022

Hey @Licht-T, I'm sorry to say that there's been no new developments. My current stance is that I should probably close the current PR and create a new fix that solves this issue with input tracking, per @jdisanti 's comment.

@jmklix jmklix added the p2 This is a standard priority issue label Nov 14, 2022
@Velfi
Copy link
Contributor

Velfi commented Nov 16, 2022

Sorry again for the lack of movement on this. This week I'm going to check if a recent PR that changes how we handle values like StartRange and EndRange fixes this.

@Velfi
Copy link
Contributor

Velfi commented Nov 17, 2022

After looking into this, I believe it's going to be blocked until we implement smithy IDL v2 support. This is due to the fact that we need to interact with the default trait which is only available in IDL v2.

@Velfi Velfi added the blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. label Nov 17, 2022
@Velfi
Copy link
Contributor

Velfi commented Nov 28, 2022

Blocked on #536 and smithy-lang/smithy-rs#1767

@rcoh
Copy link
Contributor

rcoh commented Aug 8, 2023

This should be fixed when S3 adds @clientOptional to all primitives.

@rcoh rcoh added pending-release This issue will be fixed by an approved PR that hasn't been released yet. and removed blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. labels Nov 20, 2023
@rcoh
Copy link
Contributor

rcoh commented Nov 20, 2023

This should be fixed in v0.38, we are verifying

@jdisanti
Copy link
Contributor

jdisanti commented Dec 6, 2023

Confirmed this is fixed in the 1.x SDK version.

@jdisanti jdisanti closed this as completed Dec 6, 2023
@jdisanti jdisanti removed the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Dec 6, 2023
Copy link

github-actions bot commented Dec 6, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

5 participants