Skip to content

Conversation

@10zingpd
Copy link

No description provided.

@10zingpd 10zingpd requested a review from abe-winter November 12, 2025 19:20
@10zingpd
Copy link
Author

outline:
change CreatePartialPath so it returns a .part path and a .etag path
above L198, do a head request and get the etag value
in the if statement at 198, if the file exists, load the etag file and check it against the one you just downloaded
if it's a mismatch, delete the old .part file and save the new .etag file
if the file doesn't exist, save a new etag file


func TestPartialPath(t *testing.T) {
path := CreatePartialPath("https://storage.googleapis.com/packages.viam.com/apps/viam-server/viam-server-latest-x86_64")
path, _ := CreatePartialPath("https://storage.googleapis.com/packages.viam.com/apps/viam-server/viam-server-latest-x86_64")
Copy link
Member

Choose a reason for hiding this comment

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

on L395 below, can you add another line testing the etag path as well as the partial path?

utils/utils.go Outdated

return path.Join(ViamDirs.Partials, hashString(rawURL, 7), last(strings.Split(urlPath, "/"), "")+".part")
partPath = path.Join(ViamDirs.Partials, hashString(rawURL, 7), last(strings.Split(urlPath, "/"), "")+".part")
etagPath = partPath + ".etag"
Copy link
Member

Choose a reason for hiding this comment

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

here, can you do something like

basePath := path.Join(whatever)
partPath = basePath + ".part"
etagPath = basePath + ".etag"

(so that the etag isn't .part.etag)

10zingpd and others added 6 commits November 18, 2025 15:27
- Modified CreatePartialPath to return both .part and .etag paths
- Added HEAD request to check remote ETag before downloading
- Store ETag when starting downloads and check it when resuming
- Delete partial file and start fresh if ETag mismatch detected
- Updated tests to verify both partial and ETag path lengths
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.

3 participants