Subject: Re: [bug#45409] [PATCH 3/3] guix: Split (guix substitute) from (guix scripts substitute).

From: Christopher Baines <[email-masked]>
Date: Sun, 03 Jan 2021 18:19:26 +0000

Ludovic Courtès <[email-masked]> writes:

> Hi,
>
> Christopher Baines <[email-masked]> skribis:
>
>> This means there's a module for working with substitutes, rather than all the
>> code sitting in the script. The need for this can be seen with the weather and
>> challenge scripts, that now don't have to use code from the substitute script,
>> but can instead use the substitute module.
>>
>> The separation here between the actual functionality of the substitute script
>> and the underlying functionality used both there and elsewhere should make
>> maintenance easier moving forward.
>>
>> This commit just moves code, none of the code should have been changed
>> significantly.
>
> It would still be nice to list the identifiers that were moved in the
> commit log, it’s boring :-) but it can be helpful when browsing the
> history.

Sure, I've done that now, I'd got bored by this point before.

> As for the split, I wouldn’t put as much into (guix substitutes) (I’d
> use “substitutes”, plural, for consistency with most other modules.)

Done.

> As a rule of thumb, I would keep in (guix scripts substitute) anything
> that’s very much biased towards a single short-lived process: connection
> cache, host name resolution failure cache, etc.  These things are a bit
> hacky and not designed for use as a library.  They’re also very much
> policy rather than mechanism, and as such they don’t belong in a proper
> library IMO.

I think that's fine, but it's harder said than done. I think the
connection caching and host name resolution failure caching code would
need unpicking from the general substitute fetching code, and I haven't
attempted to do that yet.

>> -(define* (http-multiple-get base-uri proc seed requests
>> -                            #:key port (verify-certificate? #t)
>> -                            (open-connection guix:open-connection-for-uri)
>> -                            (keep-alive? #t)
>> -                            (batch-size 1000))
>
> How about moving this one to (guix http-client), as a separate patch?
> I think it’s a better fit and could be useful elsewhere.

Sure, that sounds good, I'll look at it later with a separate patch.