Re: [PATCH 1/2] import: json: Silence json-fetch output.

[Ludovic Courtès]

Eric Bavier <address@hidden> skribis:

> On Wed, 07 Dec 2016 11:59:10 +0100
> address@hidden (Ludovic Courtès) wrote:
>
>> Eric Bavier <address@hidden> skribis:
>> 
>> > * guix/import/json.scm (json-fetch): Use http-fetch instead of url-fetch
>> > to avoid writing to stdout and a temporary file for each invocation.
>> > * guix/import/gem.scm (rubygems-fetch): Do not redirect json-fetch
>> > output to /dev/null.
>> > * guix/import/pypi.scm (pypi-fetch): Likewise.  
>> 
>> [...]
>> 
>> >  (define (json-fetch url)
>> >    "Return an alist representation of the JSON resource URL, or #f on 
>> > failure."
>> > -  (call-with-temporary-output-file
>> > -   (lambda (temp port)
>> > -     (and (url-fetch url temp)
>> > -          (hash-table->alist
>> > -           (call-with-input-file temp json->scm))))))
>> > +  (and=> (false-if-exception (http-fetch url))
>> > +         (lambda (port)
>> > +           (let ((result (hash-table->alist (json->scm port))))
>> > +             (close-port port)
>> > +             result))))  
>> 
>> It’d be better to not catch exceptions raised by ‘http-fetch’ here.
>> Instead they’d be caught at the top level and a detailed error message
>> would be displayed, which is always better than silently ignoring
>> issues.
>> 
>> However we’d need to check if there are uses where this is a problem.
>> For example, there might be updaters or importers that assume that #f
>> means that the package doesn’t exist or something like that.
>> 
>> WDYT?
>
> The importers that use json-fetch all do something like '(and=> meta
> ->package)', so a #f result from json-fetch is passed up to (@ (guix

OK.

So if we instead “let the exception through”, ‘guix import’ would
something like “failed to download from http://…: 404 (Not Found)”,
which is worse than “package not found in repo” in this case.

A middle ground would be this:

  (guard (c ((http-get-error? c)
             (if (= 404 (http-get-error-code c))
                 #f ;this is “expected”, just means the package isn’t known
                 (raise c)))) ;using (srfi srfi-34) here
    (let* ((port (json->scm port)))
      …))

That way, 404 would still be treated as an expected error meaning
“package does not exist in upstream repo”, but more problematic errors
(DNS lookup errors, 403, etc.) would go through.

How does that sound?

Thanks,
Ludo’.