Skip to content

Add checksum support to pelican object put#2415

Merged
jhiemstrawisc merged 11 commits into
PelicanPlatform:mainfrom
patrickbrophy:checksum-puts
Jul 10, 2025
Merged

Add checksum support to pelican object put#2415
jhiemstrawisc merged 11 commits into
PelicanPlatform:mainfrom
patrickbrophy:checksum-puts

Conversation

@patrickbrophy

Copy link
Copy Markdown
Contributor

This PR addresses issue #2231. This PR introduces the --checksum flag to the pelican object put command. This PR efficiently calculates the checksum on the fly as the object is being uploaded. Once the object is uploaded we will fetch the checksum from the Origin and then compare the locally calculated checksum and the server provided checksum. By default we use the crc32c algorithm to calculate the checksum.

@patrickbrophy patrickbrophy requested a review from turetske June 18, 2025 15:48
@patrickbrophy patrickbrophy added enhancement New feature or request client Issue affecting the OSDF client labels Jun 18, 2025
@patrickbrophy patrickbrophy linked an issue Jun 18, 2025 that may be closed by this pull request
7 tasks

@bbockelm bbockelm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fairly minor changes requested; the overall approach is sound.

Comment thread client/handle_http.go Outdated
Comment thread client/handle_http.go Outdated
Comment thread client/handle_http.go
Comment thread client/handle_http_test.go Outdated
requireChecksum: true,
requestedChecksums: []ChecksumType{AlgCRC32C},
}
transferResult, err := uploadObject(transfer)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Interesting - how does this work? It looks like the mock server only responds to HEAD requests ... I would have assumed there was a failure prior to getting to the checksum code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

After examining the internals of Go’s HTTP server, I found that if no header has been explicitly written to the response, it defaults to sending an HTTP 200 status. This behavior occurs because the test server only sets a header for HEAD requests, so other HTTP methods automatically receive a 200 response by default.

Comment thread client/handle_http_test.go
Comment thread cmd/object_put.go Outdated
@patrickbrophy patrickbrophy requested a review from bbockelm June 20, 2025 17:32
@patrickbrophy patrickbrophy added this to the v7.18 milestone Jun 23, 2025

@turetske turetske left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A few requests, mostly for clarification comments and expanded test cases.

Comment thread client/handle_http.go
Comment thread client/handle_http_test.go
Comment thread cmd/object_put.go
The reviewer requested some comments and changes to the help text. The
reviwer also requested test coverage for when there is a mismatch in the
checksums.
@patrickbrophy patrickbrophy requested a review from turetske July 9, 2025 19:09
@jhiemstrawisc

Copy link
Copy Markdown
Member

Using super powers to merge since review was handed off from BrianB to Emma and she's given the green light.

@jhiemstrawisc jhiemstrawisc merged commit 89634f2 into PelicanPlatform:main Jul 10, 2025
25 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

client Issue affecting the OSDF client enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add checksum support to client for PUTs

4 participants