-
Notifications
You must be signed in to change notification settings - Fork 200
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
Don't add etag/last_modified headers if CHECK_FILESIZE_ONLY == True #866
Conversation
Seems reasonable. Could you include verbose output showing an affected recipe running before and after the change? |
Yeah, I can do this as well. 👍 |
Here are three verbose ( First run: successfully downloads the file and stores the etag/last modified date as expected.
Second run, without fix: Even though
Third run, with fix applied: etag/last modified headers are not sent, but file size is checked and confirmed the same -- no errors, existing file is used, and no reported "new download."
|
So, this may be an extremely rare issue...but came across it on the recipe
com.github.moofit-recipes.download.MirrorOp
and I described it in autopkg/moofit-recipes#126.To re-cap here:
The web server is not accepting, or allowing, the etag
If-None-Match
and last modifiedIf-Modified-Since
header checks. If these are included, then the web server will return a 400, but if they are not, it processes the request as expected. Not quite sure why they're not supporting this as it literally should be saving them bandwidth...When setting
CHECK_FILESIZE_ONLY
asTrue
in theURLDownloader
processor step, the parent classURLGetter
processor still adds the etag and last modified headers to the call.I'm not sure if this was an oversight or by design, but as far as I know, it hasn't affected anything else for all these years...
Also not sure if this is something that the maintainers would want to merge considering how little it seems to be affecting...but I'll at least propose it here and let you decide.