Avoid creating files when download fails #21071

omus · 2017-03-17T16:45:36Z

julia> run(`curl -vLo tzdata.tar.gz https://www.iana.org/time-zones/repository/releases/tzdata2016z.tar.gz`)
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying 192.0.32.8...
* TCP_NODELAY set
* Connected to www.iana.org (192.0.32.8) port 443 (#0)
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0* TLS 1.2 connection using TLS_RSA_WITH_AES_256_GCM_SHA384
* Server certificate: *.iana.org
* Server certificate: DigiCert SHA2 High Assurance Server CA
* Server certificate: DigiCert High Assurance EV Root CA
> GET /time-zones/repository/releases/tzdata2016z.tar.gz HTTP/1.1
> Host: www.iana.org
> User-Agent: curl/7.51.0
> Accept: */*
> 
< HTTP/1.1 404 NOT FOUND
< Date: Fri, 17 Mar 2017 16:43:07 GMT
< X-Frame-Options: SAMEORIGIN
< Content-Security-Policy: upgrade-insecure-requests
< Vary: Accept-Encoding
< Content-Length: 1092
< Cache-Control: public, s-maxage=600, max-age=3600
< Expires: Fri, 17 Mar 2017 17:43:07 GMT
< content-type: application/x-gzip
< content-encoding: identity
< Server: Apache
< Strict-Transport-Security: max-age=47304003; preload
< X-Cache-Hits: 0
< Connection: keep-alive
< 
{ [995 bytes data]
* Curl_http_done: called premature == 0
100  1092  100  1092    0     0   2879      0 --:--:-- --:--:-- --:--:--  2873
* Connection #0 to host www.iana.org left intact

julia> isfile("tzdata.tar.gz")
true

julia> rm("tzdata.tar.gz")

julia> run(`curl -vLfo tzdata.tar.gz https://www.iana.org/time-zones/repository/releases/tzdata2016z.tar.gz`)
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying 192.0.32.8...
* TCP_NODELAY set
* Connected to www.iana.org (192.0.32.8) port 443 (#0)
* TLS 1.2 connection using TLS_RSA_WITH_AES_256_GCM_SHA384
* Server certificate: *.iana.org
* Server certificate: DigiCert SHA2 High Assurance Server CA
* Server certificate: DigiCert High Assurance EV Root CA
> GET /time-zones/repository/releases/tzdata2016z.tar.gz HTTP/1.1
> Host: www.iana.org
> User-Agent: curl/7.51.0
> Accept: */*
> 
* The requested URL returned error: 404 NOT FOUND
* Curl_http_done: called premature == 1
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
* Closing connection 0
curl: (22) The requested URL returned error: 404 NOT FOUND
ERROR: failed process: Process(`curl -vLfo tzdata.tar.gz https://www.iana.org/time-zones/repository/releases/tzdata2016z.tar.gz`, ProcessExited(22)) [22]
Stacktrace:
 [1] pipeline_error(::Base.Process) at ./process.jl:687
 [2] run(::Cmd) at ./process.jl:656

julia> isfile("tzdata.tar.gz")
false

Note that wget will always create a file. We could automatically clean it up if we wrap it in a try/catch.

tkelman · 2017-03-17T18:26:03Z

without -v, doesn't -f suppress failure output?

omus · 2017-03-17T18:56:25Z

Appears the same to me

Old version:

julia> download("http://httpbin.org/status/404")
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
"/var/folders/0k/c9cpsq_x02x_j4bxj3mtx1_r0000gn/T/juliaK0MJjD"

julia> download("http://127.0.0.1/")  # No webserver running
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0curl: (7) Failed to connect to 127.0.0.1 port 80: Connection refused
ERROR: failed process: Process(`curl -o /var/folders/0k/c9cpsq_x02x_j4bxj3mtx1_r0000gn/T/juliaJAHHn1 -L http://127.0.0.1/`, ProcessExited(7)) [7]
 in pipeline_error(::Base.Process) at ./process.jl:616
 in run(::Cmd) at ./process.jl:592
 in download(::String, ::String) at ./interactiveutil.jl:527
 in download(::String) at ./interactiveutil.jl:538

PR version:

julia> download("http://httpbin.org/status/404")
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
curl: (22) The requested URL returned error: 404 NOT FOUND
ERROR: failed process: Process(`curl -L -f -o /var/folders/0k/c9cpsq_x02x_j4bxj3mtx1_r0000gn/T/juliaudH6LF http://httpbin.org/status/404`, ProcessExited(22)) [22]
Stacktrace:
 [1] pipeline_error(::Base.Process) at ./process.jl:687
 [2] run(::Cmd) at ./process.jl:656
 [3] download(::String, ::String) at ./interactiveutil.jl:597
 [4] download(::String) at ./interactiveutil.jl:608

julia> download("http://127.0.0.1")
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0curl: (7) Failed to connect to 127.0.0.1 port 80: Connection refused
ERROR: failed process: Process(`curl -L -f -o /var/folders/0k/c9cpsq_x02x_j4bxj3mtx1_r0000gn/T/juliamFBy7X http://127.0.0.1`, ProcessExited(7)) [7]
Stacktrace:
 [1] pipeline_error(::Base.Process) at ./process.jl:687
 [2] run(::Cmd) at ./process.jl:656
 [3] download(::String, ::String) at ./interactiveutil.jl:597
 [4] download(::String) at ./interactiveutil.jl:608

omus · 2017-03-17T18:57:00Z

I could make some tests using "http://httpbin.org". Where should I put these tests?

tkelman · 2017-03-18T14:46:51Z

probably a new file that doesn't run by default, but can be manually run on CI like with libgit2-online and pkg

omus · 2017-03-20T17:24:48Z

I've been unable to determine if fetch leaves empty files upon failure.

tkelman · 2017-03-20T21:44:58Z

test/download.jl

@@ -0,0 +1,20 @@
+# This file is a part of Julia. License is MIT: http://julialang.org/license
+
+@testset "download" begin


don't add the testset here, that's just noise in the output

Will remove. I was looking at libgit2-online.jl which does this.

I think I just deleted that

omus · 2017-03-21T00:59:39Z

Tests on Travis CI Linux have found an issue.

omus · 2017-03-22T14:58:04Z

Attempting to debug Travis CI Linux issues. I'm unable to reproduce the issue locally so I'll be testing against Travis.

omus · 2017-03-22T16:23:53Z

Worked through the issue. When downloading an empty file Travis CI Linux uses Curl 7.22.0 which doesn't bother to create an empty file. Newer versions of Curl (like 7.43.0 on Travis CI Mac) do create the empty file (curl/curl#183).

Additionally, Window's version of download also doesn't make empty files. At first the tests didn't show this as tempname() would pre-create the file (#9053).

The test now is now indifferent as to whether downloading an empty file will or will not create a file.

omus · 2017-03-23T21:39:58Z

Code is ready to be merged. Let me know if it needs additional changes.

tkelman · 2017-03-30T07:51:55Z

@iblis17 or @ararslan, could you check the behavior of fetch here?

ararslan · 2017-03-30T16:27:44Z

fetch does not appear to leave around files on failure.

Download failure with curl no longer create a file

9932c2b

omus changed the title ~~Download failure with curl no longer create a file~~ Download failure with curl no longer creates a file Mar 17, 2017

Only create file upon wget success

dd5a7ac

omus changed the title ~~Download failure with curl no longer creates a file~~ Avoid creating files when download fails Mar 20, 2017

StefanKarpinski approved these changes Mar 20, 2017

View reviewed changes

tkelman reviewed Mar 20, 2017

View reviewed changes

omus force-pushed the cv/download-cleanup branch from 29c73f7 to 2488ae4 Compare March 22, 2017 14:59

Add tests for download

decead1

omus force-pushed the cv/download-cleanup branch from 09aa102 to decead1 Compare March 22, 2017 16:14

StefanKarpinski approved these changes Mar 23, 2017

View reviewed changes

tkelman merged commit 980119a into master Mar 30, 2017

tkelman deleted the cv/download-cleanup branch March 30, 2017 16:30

tkelman mentioned this pull request Jul 13, 2017

file testset fails unless curl is installed (0.6.0) #22783

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid creating files when download fails #21071

Avoid creating files when download fails #21071

omus commented Mar 17, 2017

tkelman commented Mar 17, 2017

omus commented Mar 17, 2017

omus commented Mar 17, 2017

tkelman commented Mar 18, 2017

omus commented Mar 20, 2017

tkelman Mar 20, 2017

omus Mar 21, 2017

tkelman Mar 21, 2017

omus commented Mar 21, 2017 •

edited

Loading

omus commented Mar 22, 2017

omus commented Mar 22, 2017

omus commented Mar 23, 2017

tkelman commented Mar 30, 2017

ararslan commented Mar 30, 2017

		@@ -0,0 +1,20 @@
		# This file is a part of Julia. License is MIT: http://julialang.org/license

		@testset "download" begin

Avoid creating files when download fails #21071

Avoid creating files when download fails #21071

Conversation

omus commented Mar 17, 2017

tkelman commented Mar 17, 2017

omus commented Mar 17, 2017

omus commented Mar 17, 2017

tkelman commented Mar 18, 2017

omus commented Mar 20, 2017

tkelman Mar 20, 2017

Choose a reason for hiding this comment

omus Mar 21, 2017

Choose a reason for hiding this comment

tkelman Mar 21, 2017

Choose a reason for hiding this comment

omus commented Mar 21, 2017 • edited Loading

omus commented Mar 22, 2017

omus commented Mar 22, 2017

omus commented Mar 23, 2017

tkelman commented Mar 30, 2017

ararslan commented Mar 30, 2017

omus commented Mar 21, 2017 •

edited

Loading