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

Enable stat operations for specific ranges #1846

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

r-scheele
Copy link

@r-scheele r-scheele commented Jun 29, 2023

ref issue number: #1813

Stat() was designed to always work on the original file size, and not a portion of the object

Copy link
Contributor

@klauspost klauspost left a comment

Choose a reason for hiding this comment

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

LGTM

api-get-object.go Outdated Show resolved Hide resolved
Copy link
Contributor

@klauspost klauspost left a comment

Choose a reason for hiding this comment

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

Thanks! That is more in line with the change I expected.

@harshavardhana
Copy link
Member

Please fix the tests @r-scheele

@klauspost
Copy link
Contributor

2023-07-06T11:10:48.3248895Z === RUN   TestCoreCopyObject
2023-07-06T11:10:48.3294758Z ##[error]    core_test.go:472: Error: number of bytes in stat does not match, want 32768, got 0
2023-07-06T11:10:48.3304410Z --- FAIL: TestCoreCopyObject (0.03s)
2023-07-06T11:10:48.3304745Z === RUN   TestCoreCopyObjectPart
2023-07-06T11:10:48.3305160Z --- PASS: TestCoreCopyObjectPart (0.41s)
2023-07-06T11:10:48.3305442Z === RUN   TestCorePutObject
2023-07-06T11:10:48.3306219Z ##[error]    core_test.go:737: Error: number of bytes in stat does not match, want 32768, got 0
2023-07-06T11:10:48.3306961Z --- FAIL: TestCorePutObject (0.03s)

@r-scheele
Copy link
Author

Hi @harshavardhana @klauspost please review again

delete(opts.headers, "Range")
objectInfo, err = c.StatObject(gctx, bucketName, objectName, StatObjectOptions(opts))
if err != nil {
_, rangeHeaderPresent := opts.headers["Range"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_, rangeHeaderPresent := opts.headers["Range"]
_, ok := opts.headers["Range"]

objectInfo, err = c.StatObject(gctx, bucketName, objectName, StatObjectOptions(opts))
if err != nil {
_, rangeHeaderPresent := opts.headers["Range"]
if rangeHeaderPresent {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if rangeHeaderPresent {
if ok {

}
} else {
// The range header is not present, continue with the existing objectInfo.
delete(opts.headers, "Range")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
delete(opts.headers, "Range")

Not needed in else {

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.

5 participants