GNU bug report logs - #77019
[PATCH 0/1] machine: hetzner: Allow attaching existing public IPs.

Previous Next

Full log

Message #17 received at 77019 <at> debbugs.gnu.org (full text, mbox):

From: Roman Scherer <roman.scherer <at> burningswell.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Roman Scherer <roman <at> burningswell.com>, Sergey Trofimov <sarg <at> sarg.org.ru>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 77019 <at> debbugs.gnu.org
Subject: Re: bug#77019: [PATCH 0/1] machine: hetzner: Allow attaching
 existing public IPs.
Date: Fri, 18 Apr 2025 13:13:58 +0200

[Message part 1 (text/plain, inline)]

Hi Ludovic and Sergey,

the patch looks good to me. Thanks for adding the tests.

I would have expected #t, #f or a string as the value of
hetzner-configuration-ipv4 and hetzner-configuration-ipv6.

It's a pitty the null issue in guile-json has no comments yet.

I would say, let's merge it. The default behaviour right now is to
enable ipv4 and ipv6 and this patch does the same. Once the issue in
guile-json has been fixed we gain the ability to disable via #f, right?

Thanks, Roman.

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi,
>
> Sergey Trofimov <sarg <at> sarg.org.ru> writes:
>
>> * gnu/machine/hetzner.scm (hetzner-configuration): Add ipv4 and ipv6
>> fields. Export accessors.
>> * gnu/machine/hetzner/http.scm (hetnzer-api-primary-ips): New function.
>> (<hetzner-primary-ip>): New json mapping.
>> (hetzner-api-server-create): Pass IP addresses in request.
>> * doc/guix.texi: Document it.
>                  ^
> Please specify the name of the node that is modified, for this list
> bullet:
>
>   * doc/guix.texi (Invoking guix deploy): Document it.
>
>> +@item @code{ipv4} (default: @code{'create})
>> +When false, no public IPv4 address is going to be attached. Specify the
>> +name of an existing primary ip to attach it to the machine. Other values
>> +would create a new address automatically.
>> +
>> +@item @code{ipv6} (default: @code{'create})
>> +When false, no public IPv6 address is going to be attached. Specify the
>> +name of an existing primary ip to attach it to the machine. Other values
>> +would create a new address automatically.
>
> To avoid repetition, use @itemx like so:
>
>   @item @code{ipv4} (default: @code{'create})
>   @itemx @code{ipv6} (default: @code{'create})
>   When false, no public IPv4 (respectively IPv6) address is attached. …
>
> Also, please leave to spaces after end-of-sentence periods and
> capitalize acronyms like “IP”.
>
>> +  (ipv4 hetzner-configuration-ipv4
>> +        (default 'create))
>> +  (ipv6 hetzner-configuration-ipv6
>> +        (default 'create))
>
> Am I right that 'create doesn’t have any special meaning?  In that case,
> it seems to be that it should be either #f or a string?  Or #f or string
> or #t?  This should be documented.
>
>> +(define-json-mapping <hetzner-primary-ip>
>
> Please add a short comment above explaining what this is, possibly
> linking to the relevant Hetzner doc.
>
> The rest LGTM at first sight but I know nothing about Hetzner so I’d
> prefer if Roman could chime in.
>
> Thanks!
>
> Ludo’.

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.