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.

Sign up for GitHub

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

Jump to bottom

Use a HEAD instead of GET #180

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Use a HEAD instead of GET #180

merged 1 commit into from
Feb 20, 2024

Conversation

votdev
Copy link
Contributor

@votdev votdev commented Feb 8, 2024 •
edited
Loading

... to request only the headers of the resource without the body.

Which issue(s) this PR fixes:

Issue #7892

What this PR does / why we need it:

Use a HEAD instead of a GET request because in that situation it is the correct method that should be used. The GetSizeFromURL function is processing only the Content-Length header and no data from the body.

Special notes for your reviewer:

Additional documentation or context

@votdev votdev self-assigned this Feb 8, 2024
@votdev votdev force-pushed the issue_7892_head_instead_get branch from 1dcdaf3 to b79d7a4 Compare February 8, 2024 15:49
@votdev votdev added the enhancement New feature or request label Feb 8, 2024
@votdev votdev requested a review from innobead February 8, 2024 16:04
@votdev votdev force-pushed the issue_7892_head_instead_get branch 2 times, most recently from e503e55 to 38eadb9 Compare February 9, 2024 15:44
@innobead
Copy link
Member

@votdev Please wrap up this PR.

@votdev votdev marked this pull request as ready for review February 19, 2024 07:48
@votdev votdev requested a review from a team as a code owner February 19, 2024 07:48
@votdev
Use a HEAD instead of GET
c7c1acb 
Even if Go only downloads the body for a GET request when the body-reader is used; it is still wrong and a HEAD request should be used instead.

Additionaly some code polishing is done.

Signed-off-by: Volker Theile <[email protected]>
@innobead innobead merged commit c44ba3b into longhorn:master Feb 20, 2024
6 checks passed
@innobead
Copy link
Member

@mergify backport v1.6.x v1.5.x

@mergify Mergify
Copy link

mergify bot commented Feb 20, 2024 •
edited
Loading

backport v1.6.x v1.5.x

✅ Backports have been created

This was referenced Feb 20, 2024
Merged
Merged
@votdev votdev deleted the issue_7892_head_instead_get branch February 20, 2024 08:42
@votdev
Copy link
Contributor Author

votdev commented Feb 20, 2024 •
edited
Loading

Testcase

It can not be tested whether a GET or a HEAD is used internally, but we can test the outcome to check nothing is broken.

Go to the Backing Image tab and press Create Backing Image. Use the following values:

Name: test01
Create From: Download from URL
URL: https://download.opensuse.org/distribution/leap/15.0/iso/openSUSE-Leap-15.0-NET-x86_64.iso
Expected Checksum: 0975605d33ffcbd806f28d9c1eb176c17fe7878c6cd07f985b2bfcd3f72d7a71c389de1e7784803a1241cb9d9bff4cf5733bd333f5a5ca029282c71ea7754621

Press OK. In the list view you can click on the name test01. You will see the download progress. If it is finished, there should be a Verified badge in the checksum field.

grafik

The image size should be ~ 118MiB.

grafik

This was referenced Feb 20, 2024
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@innobead innobead innobead approved these changes

@ChanYiLin ChanYiLin Awaiting requested review from ChanYiLin

@shuo-wu shuo-wu Awaiting requested review from shuo-wu

Assignees

@votdev votdev

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@votdev @innobead