diff mbox series

[swugenerator] Handle multiple configuration files

Message ID 20250612113443.2220579-1-ernestas.k@iconn-networks.com
State Accepted
Delegated to: Stefano Babic
Headers show
Series [swugenerator] Handle multiple configuration files | expand

Commit Message

Ernestas Kulik June 12, 2025, 11:34 a.m. UTC
Currently, only a single configuration file is allowed, but there are
cases where spreading configuration across several files is difficult to
avoid (e.g. generated version variables and static configuration).

To accommodate such workflows, this commit implements a custom argparse
action to update the config dictionary with values from subsequent
files.

Signed-off-by: Ernestas Kulik <ernestas.k@iconn-networks.com>
---
 swugenerator/main.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Stefano Babic June 16, 2025, 7:17 a.m. UTC | #1
Hi Ernestas,

On 12.06.25 13:34, 'Ernestas Kulik' via swupdate wrote:
> Currently, only a single configuration file is allowed, but there are
> cases where spreading configuration across several files is difficult to
> avoid (e.g. generated version variables and static configuration).
> 
> To accommodate such workflows, this commit implements a custom argparse
> action to update the config dictionary with values from subsequent
> files.
> 
> Signed-off-by: Ernestas Kulik <ernestas.k@iconn-networks.com>
> ---
>   swugenerator/main.py | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/swugenerator/main.py b/swugenerator/main.py
> index 1989c53..cf66a13 100644
> --- a/swugenerator/main.py
> +++ b/swugenerator/main.py
> @@ -27,6 +27,15 @@ class InvalidSigningOption(ValueError):
>       """Raised when an invalid signing option is passed via command line"""
>   
>   
> +class UpdateAction(argparse.Action):
> +    def __call__(self, parser, namespace, values, option_string=None):
> +        cfg = getattr(namespace, self.dest, None)
> +        if cfg is None:
> +            cfg = {}
> +        cfg.update(*values)
> +        setattr(namespace, self.dest, cfg)
> +
> +
>   def extract_keys(keyfile: str) -> Tuple[Optional[str], Optional[str]]:
>       """Extracts encryption key and initialization vector (IV)
>   
> @@ -300,10 +309,13 @@ def parse_args(args: List[str]) -> None:
>           help="SWU output file",
>       )
>   
> +    parser.register("action", "update", UpdateAction)
>       parser.add_argument(
>           "-c",
>           "--config",
>           default={},
> +        action="update",
> +        nargs="*",
>           type=parse_config_file,
>           help="configuration file",
>       )

Reviewed-by : Stefano Babic <stefano.babic@swupdate.org>
Pratik Manvar June 23, 2025, 9:31 a.m. UTC | #2
Hi Stefano,

Is this change <https://groups.google.com/g/swupdate/c/YFVDAphzisE> something 
related to below pending PRs?
https://github.com/sbabic/swugenerator/pull/5
https://github.com/sbabic/swugenerator/pull/16

Thanks & Regards,
Pratik Mavnar
On Monday, June 16, 2025 at 12:47:33 PM UTC+5:30 Stefano Babic wrote:

> Hi Ernestas,
>
> On 12.06.25 13:34, 'Ernestas Kulik' via swupdate wrote:
> > Currently, only a single configuration file is allowed, but there are
> > cases where spreading configuration across several files is difficult to
> > avoid (e.g. generated version variables and static configuration).
> > 
> > To accommodate such workflows, this commit implements a custom argparse
> > action to update the config dictionary with values from subsequent
> > files.
> > 
> > Signed-off-by: Ernestas Kulik <ernes...@iconn-networks.com>
> > ---
> > swugenerator/main.py | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> > 
> > diff --git a/swugenerator/main.py b/swugenerator/main.py
> > index 1989c53..cf66a13 100644
> > --- a/swugenerator/main.py
> > +++ b/swugenerator/main.py
> > @@ -27,6 +27,15 @@ class InvalidSigningOption(ValueError):
> > """Raised when an invalid signing option is passed via command line"""
> > 
> > 
> > +class UpdateAction(argparse.Action):
> > + def __call__(self, parser, namespace, values, option_string=None):
> > + cfg = getattr(namespace, self.dest, None)
> > + if cfg is None:
> > + cfg = {}
> > + cfg.update(*values)
> > + setattr(namespace, self.dest, cfg)
> > +
> > +
> > def extract_keys(keyfile: str) -> Tuple[Optional[str], Optional[str]]:
> > """Extracts encryption key and initialization vector (IV)
> > 
> > @@ -300,10 +309,13 @@ def parse_args(args: List[str]) -> None:
> > help="SWU output file",
> > )
> > 
> > + parser.register("action", "update", UpdateAction)
> > parser.add_argument(
> > "-c",
> > "--config",
> > default={},
> > + action="update",
> > + nargs="*",
> > type=parse_config_file,
> > help="configuration file",
> > )
>
> Reviewed-by : Stefano Babic <stefan...@swupdate.org>
>
Pratik Manvar June 24, 2025, 6:05 a.m. UTC | #3
Apologies for jumping into the wrong conversation, @Kulik and @Stefano.
On Monday, June 23, 2025 at 3:01:52 PM UTC+5:30 Pratik Manvar wrote:

> Hi Stefano,
>
> Is this change <https://groups.google.com/g/swupdate/c/YFVDAphzisE> something 
> related to below pending PRs?
> https://github.com/sbabic/swugenerator/pull/5
> https://github.com/sbabic/swugenerator/pull/16
>
> Thanks & Regards,
> Pratik Mavnar
> On Monday, June 16, 2025 at 12:47:33 PM UTC+5:30 Stefano Babic wrote:
>
>> Hi Ernestas, 
>>
>> On 12.06.25 13:34, 'Ernestas Kulik' via swupdate wrote: 
>> > Currently, only a single configuration file is allowed, but there are 
>> > cases where spreading configuration across several files is difficult 
>> to 
>> > avoid (e.g. generated version variables and static configuration). 
>> > 
>> > To accommodate such workflows, this commit implements a custom argparse 
>> > action to update the config dictionary with values from subsequent 
>> > files. 
>> > 
>> > Signed-off-by: Ernestas Kulik <ernes...@iconn-networks.com> 
>> > --- 
>> > swugenerator/main.py | 12 ++++++++++++ 
>> > 1 file changed, 12 insertions(+) 
>> > 
>> > diff --git a/swugenerator/main.py b/swugenerator/main.py 
>> > index 1989c53..cf66a13 100644 
>> > --- a/swugenerator/main.py 
>> > +++ b/swugenerator/main.py 
>> > @@ -27,6 +27,15 @@ class InvalidSigningOption(ValueError): 
>> > """Raised when an invalid signing option is passed via command line""" 
>> > 
>> > 
>> > +class UpdateAction(argparse.Action): 
>> > + def __call__(self, parser, namespace, values, option_string=None): 
>> > + cfg = getattr(namespace, self.dest, None) 
>> > + if cfg is None: 
>> > + cfg = {} 
>> > + cfg.update(*values) 
>> > + setattr(namespace, self.dest, cfg) 
>> > + 
>> > + 
>> > def extract_keys(keyfile: str) -> Tuple[Optional[str], Optional[str]]: 
>> > """Extracts encryption key and initialization vector (IV) 
>> > 
>> > @@ -300,10 +309,13 @@ def parse_args(args: List[str]) -> None: 
>> > help="SWU output file", 
>> > ) 
>> > 
>> > + parser.register("action", "update", UpdateAction) 
>> > parser.add_argument( 
>> > "-c", 
>> > "--config", 
>> > default={}, 
>> > + action="update", 
>> > + nargs="*", 
>> > type=parse_config_file, 
>> > help="configuration file", 
>> > ) 
>>
>> Reviewed-by : Stefano Babic <stefan...@swupdate.org> 
>>
>
Stefano Babic July 1, 2025, 8:49 a.m. UTC | #4
On 6/16/25 09:17, Stefano Babic wrote:
> Hi Ernestas,
> 
> On 12.06.25 13:34, 'Ernestas Kulik' via swupdate wrote:
>> Currently, only a single configuration file is allowed, but there are
>> cases where spreading configuration across several files is difficult to
>> avoid (e.g. generated version variables and static configuration).
>>
>> To accommodate such workflows, this commit implements a custom argparse
>> action to update the config dictionary with values from subsequent
>> files.
>>
>> Signed-off-by: Ernestas Kulik <ernestas.k@iconn-networks.com>
>> ---
>>   swugenerator/main.py | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/swugenerator/main.py b/swugenerator/main.py
>> index 1989c53..cf66a13 100644
>> --- a/swugenerator/main.py
>> +++ b/swugenerator/main.py
>> @@ -27,6 +27,15 @@ class InvalidSigningOption(ValueError):
>>       """Raised when an invalid signing option is passed via command 
>> line"""
>> +class UpdateAction(argparse.Action):
>> +    def __call__(self, parser, namespace, values, option_string=None):
>> +        cfg = getattr(namespace, self.dest, None)
>> +        if cfg is None:
>> +            cfg = {}
>> +        cfg.update(*values)
>> +        setattr(namespace, self.dest, cfg)
>> +
>> +
>>   def extract_keys(keyfile: str) -> Tuple[Optional[str], Optional[str]]:
>>       """Extracts encryption key and initialization vector (IV)
>> @@ -300,10 +309,13 @@ def parse_args(args: List[str]) -> None:
>>           help="SWU output file",
>>       )
>> +    parser.register("action", "update", UpdateAction)
>>       parser.add_argument(
>>           "-c",
>>           "--config",
>>           default={},
>> +        action="update",
>> +        nargs="*",
>>           type=parse_config_file,
>>           help="configuration file",
>>       )
> 
> Reviewed-by : Stefano Babic <stefano.babic@swupdate.org>
> 

Applied to main, thanks !

Best regards,
Stefano Babic
James Hilliard Sept. 17, 2025, 9:04 p.m. UTC | #5
On Thu, Jun 12, 2025 at 5:55 AM 'Ernestas Kulik' via swupdate
<swupdate@googlegroups.com> wrote:
>
> Currently, only a single configuration file is allowed, but there are
> cases where spreading configuration across several files is difficult to
> avoid (e.g. generated version variables and static configuration).
>
> To accommodate such workflows, this commit implements a custom argparse
> action to update the config dictionary with values from subsequent
> files.
>
> Signed-off-by: Ernestas Kulik <ernestas.k@iconn-networks.com>
> ---
>  swugenerator/main.py | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/swugenerator/main.py b/swugenerator/main.py
> index 1989c53..cf66a13 100644
> --- a/swugenerator/main.py
> +++ b/swugenerator/main.py
> @@ -27,6 +27,15 @@ class InvalidSigningOption(ValueError):
>      """Raised when an invalid signing option is passed via command line"""
>
>
> +class UpdateAction(argparse.Action):
> +    def __call__(self, parser, namespace, values, option_string=None):
> +        cfg = getattr(namespace, self.dest, None)
> +        if cfg is None:
> +            cfg = {}
> +        cfg.update(*values)
> +        setattr(namespace, self.dest, cfg)
> +
> +
>  def extract_keys(keyfile: str) -> Tuple[Optional[str], Optional[str]]:
>      """Extracts encryption key and initialization vector (IV)
>
> @@ -300,10 +309,13 @@ def parse_args(args: List[str]) -> None:
>          help="SWU output file",
>      )
>
> +    parser.register("action", "update", UpdateAction)
>      parser.add_argument(
>          "-c",
>          "--config",
>          default={},
> +        action="update",
> +        nargs="*",

I think this is causing some weird breakage as it makes the command line
arg behavior dependent upon the order in which the command line args are
specified.

For example if I pass --config with 1 file path argument immediately
prior to the "create", then the "create" command unexpectedly gets
interpreted as a filepath. If I move args around so that the "create"
command is not immediately after the --config argument then it works
correctly.

It's not clear if there's a proper way to fix this issue.

>          type=parse_config_file,
>          help="configuration file",
>      )
> --
> 2.47.2
>
> --
> You received this message because you are subscribed to the Google Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to swupdate+unsubscribe@googlegroups.com.
> To view this discussion visit https://groups.google.com/d/msgid/swupdate/20250612113443.2220579-1-ernestas.k%40iconn-networks.com.
Stefano Babic Sept. 18, 2025, 3:13 a.m. UTC | #6
Hi James, Ernestas,

On 9/17/25 23:04, James Hilliard wrote:
> On Thu, Jun 12, 2025 at 5:55 AM 'Ernestas Kulik' via swupdate
> <swupdate@googlegroups.com> wrote:
>>
>> Currently, only a single configuration file is allowed, but there are
>> cases where spreading configuration across several files is difficult to
>> avoid (e.g. generated version variables and static configuration).
>>
>> To accommodate such workflows, this commit implements a custom argparse
>> action to update the config dictionary with values from subsequent
>> files.
>>
>> Signed-off-by: Ernestas Kulik <ernestas.k@iconn-networks.com>
>> ---
>>   swugenerator/main.py | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/swugenerator/main.py b/swugenerator/main.py
>> index 1989c53..cf66a13 100644
>> --- a/swugenerator/main.py
>> +++ b/swugenerator/main.py
>> @@ -27,6 +27,15 @@ class InvalidSigningOption(ValueError):
>>       """Raised when an invalid signing option is passed via command line"""
>>
>>
>> +class UpdateAction(argparse.Action):
>> +    def __call__(self, parser, namespace, values, option_string=None):
>> +        cfg = getattr(namespace, self.dest, None)
>> +        if cfg is None:
>> +            cfg = {}
>> +        cfg.update(*values)
>> +        setattr(namespace, self.dest, cfg)
>> +
>> +
>>   def extract_keys(keyfile: str) -> Tuple[Optional[str], Optional[str]]:
>>       """Extracts encryption key and initialization vector (IV)
>>
>> @@ -300,10 +309,13 @@ def parse_args(args: List[str]) -> None:
>>           help="SWU output file",
>>       )
>>
>> +    parser.register("action", "update", UpdateAction)
>>       parser.add_argument(
>>           "-c",
>>           "--config",
>>           default={},
>> +        action="update",
>> +        nargs="*",
> 
> I think this is causing some weird breakage as it makes the command line
> arg behavior dependent upon the order in which the command line args are
> specified.
> 
> For example if I pass --config with 1 file path argument immediately
> prior to the "create", then the "create" command unexpectedly gets
> interpreted as a filepath. If I move args around so that the "create"
> command is not immediately after the --config argument then it works
> correctly.
> 
> It's not clear if there's a proper way to fix this issue.

Thanks for reporting this. It is maybe time to rethink the feature. 
Generally speaking, having a lot of different configuration files is 
something unusual, specially if they have the same meaning. The 
configuration file has libconfig syntax, and in libconfig it is possible 
to include other files, for example:

variables : {
	@include "file1"
	@include "file2"
          .....
}

so we can still passing a single file as before, and the libconfig 
parser will do the rest. What do you think ?

Best regards,
Stefano

> 
>>           type=parse_config_file,
>>           help="configuration file",
>>       )
>> --
>> 2.47.2
>>
>> --
>> You received this message because you are subscribed to the Google Groups "swupdate" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to swupdate+unsubscribe@googlegroups.com.
>> To view this discussion visit https://groups.google.com/d/msgid/swupdate/20250612113443.2220579-1-ernestas.k%40iconn-networks.com.
>
Ulrich Teichert Sept. 18, 2025, 6:59 a.m. UTC | #7
Hi,


On Thursday, September 18, 2025 at 5:13:36 AM UTC+2 Stefano Babic wrote:

Hi James, Ernestas, 

[del] 

Thanks for reporting this. It is maybe time to rethink the feature. 
Generally speaking, having a lot of different configuration files is 
something unusual, specially if they have the same meaning. The 
configuration file has libconfig syntax, and in libconfig it is possible 
to include other files, for example: 

variables : { 
@include "file1" 
@include "file2" 
..... 
} 

so we can still passing a single file as before, and the libconfig 
parser will do the rest. What do you think ? 

[del]
Actually, that's what I am using to cobble together some complex 
configurations.
I see no need for more than one configuration file,

just my 2 ¢,
Uli
Ernestas Kulik Sept. 18, 2025, 7:28 a.m. UTC | #8
On Thu, 2025-09-18 at 05:13 +0200, Stefano Babic wrote:
> Hi James, Ernestas,

Hi,

> Thanks for reporting this. It is maybe time to rethink the feature. 
> Generally speaking, having a lot of different configuration files is 
> something unusual, specially if they have the same meaning. The 
> configuration file has libconfig syntax, and in libconfig it is
> possible 
> to include other files, for example:
> 
> variables : {
> 	@include "file1"
> 	@include "file2"
>           .....
> }
> 
> so we can still passing a single file as before, and the libconfig 
> parser will do the rest. What do you think ?

My use case was that the secondary config is generated at build time
(without a stable path), so I cannot use includes without mangling
files.

As for the issue itself, the fix is a couple of lines, so I can either
send a patch or we can revert the change. I’m not particularly bothered
with either option, since it’s just about convenience. :)
Stefano Babic Sept. 18, 2025, 7:46 a.m. UTC | #9
Hi Ernestas,

On 9/18/25 09:28, 'Ernestas Kulik' via swupdate wrote:
> On Thu, 2025-09-18 at 05:13 +0200, Stefano Babic wrote:
>> Hi James, Ernestas,
> 
> Hi,
> 
>> Thanks for reporting this. It is maybe time to rethink the feature.
>> Generally speaking, having a lot of different configuration files is
>> something unusual, specially if they have the same meaning. The
>> configuration file has libconfig syntax, and in libconfig it is
>> possible
>> to include other files, for example:
>>
>> variables : {
>> 	@include "file1"
>> 	@include "file2"
>>            .....
>> }
>>
>> so we can still passing a single file as before, and the libconfig
>> parser will do the rest. What do you think ?
> 
> My use case was that the secondary config is generated at build time
> (without a stable path), so I cannot use includes without mangling
> files.

That means you could have a main file with just include statements that 
you generate as well.

> 
> As for the issue itself, the fix is a couple of lines, so I can either
> send a patch or we can revert the change. I’m not particularly bothered
> with either option, since it’s just about convenience. :)
> 

Ok - I think using the includes makes the interface much cleaner, even 
if the solution here is a couple of lines. I will revert this change and 
push.

Regards,
Stefano
Ernestas Kulik Sept. 18, 2025, 7:48 a.m. UTC | #10
On Thu, 2025-09-18 at 09:46 +0200, Stefano Babic wrote:
> Hi Ernestas,
> 
> On 9/18/25 09:28, 'Ernestas Kulik' via swupdate wrote:
> > On Thu, 2025-09-18 at 05:13 +0200, Stefano Babic wrote:
> > > Hi James, Ernestas,
> > 
> > Hi,
> > 
> > > Thanks for reporting this. It is maybe time to rethink the
> > > feature.
> > > Generally speaking, having a lot of different configuration files
> > > is
> > > something unusual, specially if they have the same meaning. The
> > > configuration file has libconfig syntax, and in libconfig it is
> > > possible
> > > to include other files, for example:
> > > 
> > > variables : {
> > > 	@include "file1"
> > > 	@include "file2"
> > >            .....
> > > }
> > > 
> > > so we can still passing a single file as before, and the
> > > libconfig
> > > parser will do the rest. What do you think ?
> > 
> > My use case was that the secondary config is generated at build
> > time
> > (without a stable path), so I cannot use includes without mangling
> > files.
> 
> That means you could have a main file with just include statements
> that 
> you generate as well.

Fair point, guess I got too fixated on the solution. :)

> > As for the issue itself, the fix is a couple of lines, so I can
> > either
> > send a patch or we can revert the change. I’m not particularly
> > bothered
> > with either option, since it’s just about convenience. :)
> > 
> 
> Ok - I think using the includes makes the interface much cleaner,
> even 
> if the solution here is a couple of lines. I will revert this change
> and 
> push.

No problem, cheers!
diff mbox series

Patch

diff --git a/swugenerator/main.py b/swugenerator/main.py
index 1989c53..cf66a13 100644
--- a/swugenerator/main.py
+++ b/swugenerator/main.py
@@ -27,6 +27,15 @@  class InvalidSigningOption(ValueError):
     """Raised when an invalid signing option is passed via command line"""
 
 
+class UpdateAction(argparse.Action):
+    def __call__(self, parser, namespace, values, option_string=None):
+        cfg = getattr(namespace, self.dest, None)
+        if cfg is None:
+            cfg = {}
+        cfg.update(*values)
+        setattr(namespace, self.dest, cfg)
+
+
 def extract_keys(keyfile: str) -> Tuple[Optional[str], Optional[str]]:
     """Extracts encryption key and initialization vector (IV)
 
@@ -300,10 +309,13 @@  def parse_args(args: List[str]) -> None:
         help="SWU output file",
     )
 
+    parser.register("action", "update", UpdateAction)
     parser.add_argument(
         "-c",
         "--config",
         default={},
+        action="update",
+        nargs="*",
         type=parse_config_file,
         help="configuration file",
     )