Message ID | 20220410134114.371-1-attila@lendvai.name |
---|---|
State | New |
Headers | show |
Series | [bug#54836,v2,1/3] http-client: Added accept-all-response-codes? argument. | expand |
Hi, Attila Lendvai <attila@lendvai.name> skribis: > This is needed when dealing with golang packages, as per: > https://golang.org/ref/mod#vcs-find > > A page may return 404, but at the same time also contain the sought after > `go-import` meta tag. An example for such a project/page is: > https://www.gonum.org/v1/gonum?go-get=1 > > It's not enough to just handle the thrown exception, because we need to be > able to get hold of the fetched content, too. Would it make sense, then, to use the lower-level ‘http-get’ from (web client)? That would let the code deal with all the HTTP idiosyncrasies. Ludo’.
> > It's not enough to just handle the thrown exception, because we need to be > > able to get hold of the fetched content, too. > > Would it make sense, then, to use the lower-level ‘http-get’ from (web > client)? That would let the code deal with all the HTTP idiosyncrasies. i think it boils down to this trade-off: 1) keep http-fetch simpler, at the expense of reimplementing parts of it in the go importer (e.g. the redirection logic) 2) add this extra complexity to http-fetch, and avoid the extra complexity of a local, potentially half-assed %http-fetch in the go importer. 3) something else i'm not aware of please advise how to reshape this patch/feature, because it's needed to file my go importer patches. -- • attila lendvai • PGP: 963F 5D5F 45C7 DFCD 0A39 -- The use of power is only needed when you want to do something harmful, otherwise love is enough to get everything done.
> i think it boils down to this trade-off: > > 1) keep http-fetch simpler, at the expense of reimplementing parts of > it in the go importer (e.g. the redirection logic) > > 2) add this extra complexity to http-fetch, and avoid the extra > complexity of a local, potentially half-assed %http-fetch in the go > importer. > > 3) something else i'm not aware of > > please advise how to reshape this patch/feature, because it's needed > to file my go importer patches. can someone with authority please decide how to proceed with this? (the reason is that i'd like to file my golang importer improvements before it develops a painful merge conflict with master.) -- • attila lendvai • PGP: 963F 5D5F 45C7 DFCD 0A39 -- “Your living is determined not so much by what life brings to you as by the attitude you bring to life; not so much by what happens to you as by the way your mind looks at what happens.” — Khalil Gibran (1883–1931)
Hi, And sorry for the delay. Attila Lendvai <attila@lendvai.name> skribis: >> > It's not enough to just handle the thrown exception, because we need to be >> > able to get hold of the fetched content, too. >> >> Would it make sense, then, to use the lower-level ‘http-get’ from (web >> client)? That would let the code deal with all the HTTP idiosyncrasies. > > > i think it boils down to this trade-off: > > 1) keep http-fetch simpler, at the expense of reimplementing parts of > it in the go importer (e.g. the redirection logic) > > 2) add this extra complexity to http-fetch, and avoid the extra > complexity of a local, potentially half-assed %http-fetch in the go > importer. > > 3) something else i'm not aware of For now, I’m somewhat in favor of #1. My take would be: try to implement whatever’s needed specifically for the Go importer; from there, we can eventually revisit that situation and maybe switch to something that’s more like #2. How does that sound? Thanks, Ludo’.
Hi Attila, Ludovic Courtès <ludo@gnu.org> writes: > Hi, > > And sorry for the delay. > > Attila Lendvai <attila@lendvai.name> skribis: > >>> > It's not enough to just handle the thrown exception, because we need to be >>> > able to get hold of the fetched content, too. >>> >>> Would it make sense, then, to use the lower-level ‘http-get’ from (web >>> client)? That would let the code deal with all the HTTP idiosyncrasies. >> >> >> i think it boils down to this trade-off: >> >> 1) keep http-fetch simpler, at the expense of reimplementing parts of >> it in the go importer (e.g. the redirection logic) >> >> 2) add this extra complexity to http-fetch, and avoid the extra >> complexity of a local, potentially half-assed %http-fetch in the go >> importer. >> >> 3) something else i'm not aware of > > For now, I’m somewhat in favor of #1. > > My take would be: try to implement whatever’s needed specifically for > the Go importer; from there, we can eventually revisit that situation > and maybe switch to something that’s more like #2. > > How does that sound? I think we're missing your reworked 1/3 patch here, taking into account the above feedback from Ludo.
diff --git a/guix/http-client.scm b/guix/http-client.scm index 143ed6de31..8a5b3deecd 100644 --- a/guix/http-client.scm +++ b/guix/http-client.scm @@ -82,7 +82,8 @@ (define* (http-fetch uri #:key port (text? #f) (buffered? #t) (verify-certificate? #t) (headers '((user-agent . "GNU Guile"))) (log-port (current-error-port)) - timeout) + timeout + (accept-all-response-codes? #f)) "Return an input port containing the data at URI, and the expected number of bytes available or #f. If TEXT? is true, the data at URI is considered to be textual. Follow any HTTP redirection. When BUFFERED? is #f, return an @@ -99,7 +100,9 @@ (define* (http-fetch uri #:key port (text? #f) (buffered? #t) Write information about redirects to LOG-PORT. -Raise an '&http-get-error' condition if downloading fails." +When ACCEPT-ALL-RESPONSE-CODES? is false then raise an '&http-get-error' +condition if downloading fails, otherwise return the response regardless +of the reponse code." (define uri* (if (string? uri) (string->uri uri) uri)) @@ -150,18 +153,20 @@ (define uri* verify-certificate? #:timeout timeout))))) (else - (raise (condition (&http-get-error - (uri uri) - (code code) - (reason (response-reason-phrase resp)) - (headers (response-headers resp))) - (&message - (message - (format - #f - (G_ "~a: HTTP download failed: ~a (~s)") - (uri->string uri) code - (response-reason-phrase resp)))))))))))) + (if accept-all-response-codes? + (values data (response-content-length resp)) + (raise (condition (&http-get-error + (uri uri) + (code code) + (reason (response-reason-phrase resp)) + (headers (response-headers resp))) + (&message + (message + (format + #f + (G_ "~a: HTTP download failed: ~a (~s)") + (uri->string uri) code + (response-reason-phrase resp))))))))))))) (define-syntax-rule (false-if-networking-error exp) "Return #f if EXP triggers a network related exception as can occur when