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

Improve the error handling of the source download command #307

Merged
merged 3 commits into from
Oct 24, 2023
Merged

Improve the error handling of the source download command #307

merged 3 commits into from
Oct 24, 2023

Conversation

primeos-work
Copy link
Member

This builds upon #305 and avoids two more issues that we noticed:

  1. An unsuccessful download still creates the source file (empty) -> subsequent attempts will fail (without --force). An example:
$ export http_proxy=localhost:42
$ butido source download tree
[00:00:02] (100%): ████████████████████████████████████████ | Repository loading finished
[00:00:00] (100%): ████████████████████████████████████████ | At least one download of 1 failed
Error: source command failed

Caused by:
    0: Downloading 'http://mama.indstate.edu/users/ice/tree/src/tree-1.8.0.tgz'
    1: error sending request for url (http://mama.indstate.edu/users/ice/tree/src/tree-1.8.0.tgz): error trying to connect: tcp connect error: Connection refused (os error 111)
    2: error trying to connect: tcp connect error: Connection refused (os error 111)
    3: tcp connect error: Connection refused (os error 111)
    4: Connection refused (os error 111)
$ butido source download tree
[00:00:01] (100%): ████████████████████████████████████████ | Repository loading finished
[00:00:00] (100%): ████████████████████████████████████████ | At least one download of 0 failed
Error: source command failed

Caused by:
    Source exists: /srv/butido/sources/tree-1.8.0/src.source
$ file /srv/butido/sources/tree-1.8.0/src.source
/srv/butido/sources/tree-1.8.0/src.source: empty
  1. The HTTP status code was never checked -> if the URL is invalid, butido will happily store the HTTP 404 Not Found HTML response as the source file, e.g.:
$ butido source download tree                                                                                                                               [00:00:02] (100%): ████████████████████████████████████████ | Repository loading finished
[00:00:00] (100%): ████████████████████████████████████████ | Succeeded 1/1 downloads
$ file /srv/butido/sources/tree-1.8.0/src.source
/srv/butido/sources/tree-1.8.0/src.source: HTML document, ASCII text
$ cat /srv/butido/sources/tree-1.8.0/src.source
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>404 Not Found</title>
</head><body>
<h1>Not Found</h1>
<p>The requested URL was not found on this server.</p>
</body></html>

This now results in a fatal error instead:

$ butido source download tree
[00:00:01] (100%): ████████████████████████████████████████ | Repository loading finished
[00:00:00] (100%): ████████████████████████████████████████ | At least one download of 1 failed
Error: source command failed

Caused by:
    0: Downloading "http://mama.indstate.edu/users/ice/tree/src/tree-1.8.42.tgz" failed
    1: Received HTTP status code "404 Not Found" but "200 OK" is expected for a successful download

ammernico and others added 3 commits October 10, 2023 18:22
@ammernico @primeos-work
Warn the user when downloading HTML files
6c70f44 
Warn the user when downloading HTML files as the source download URL
might point to the website instead of a source archive.

Fixes science-computing#243

Signed-off-by: Nico Steinle <nico.steinle@atos.net>
Co-authored-by: Michael Weiss <michael.weiss@atos.net>
@primeos-work
Create the files for source downloads only after a successful request
b76e4d3 
The downloaded source files used to be created before the actual HTTP
request was made, which is problematic if the download failed as
subsequent `butido source download` attempts will fail due to the file
already existing (butido thinks that the source was already downloaded
but it's just an empty file).

Signed-off-by: Michael Weiss <michael.weiss@atos.net>
@primeos-work
Check the HTTP status code when downloading source files
1cf3d90 
We especially don't want to download the HTML response as source
file/archive when the download fails due to HTTP 404 Not Found.
Successful downloads should only result in OK (200) but we could relax
the check via `response.status()..is_success()` (checks if status is
within 200-299) if necessary (if some HTTP servers send wrong/weird
status codes).

Signed-off-by: Michael Weiss <michael.weiss@atos.net>
@christophprokop
Copy link
Collaborator

Great! Thank you!

@christophprokop christophprokop added this pull request to the merge queue Oct 24, 2023
Merged via the queue into science-computing:master with commit 352b32e Oct 24, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christophprokop christophprokop Awaiting requested review from christophprokop

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@primeos-work @christophprokop @ammernico