Hi,
This patch allows users to select the systems for which to build an iso from the webinterface (batch action). Very handy for those without PXE and many systems.
Grtz, Jörgen Maas
On Sat, Aug 20, 2011 at 4:21 AM, Jörgen Maas jorgen.maas@gmail.com wrote:
Hi,
This patch allows users to select the systems for which to build an iso from the webinterface (batch action). Very handy for those without PXE and many systems.
I would like to see this code use the input_string_or_list() function from utils.py. It has always annoyed me that buildiso is the only function that takes a list separated by commas instead of spaces.
On Sat, Aug 20, 2011 at 1:35 PM, James Cammarata jimi@sngx.net wrote: <snip>
I would like to see this code use the input_string_or_list() function from utils.py. It has always annoyed me that buildiso is the only function that takes a list separated by commas instead of spaces.
Good suggestion, i will look into this and come up with a better patch. Thanks a lot for the review!
Grtz, Jörgen Maas
Hi James, others,
If i adjust the code to something like:
if systems is not None: # this check isn't really needed anymore since it's done in the utils function aswell which_systems = utils.input_string_or_list(systems) else: which_systems = []
The behaviour from the CLI will be like this: [root@daemon /]# cobbler buildiso --systems="x doosje"
Which is ok by me, but it isn't very consistent with other CLI commands, e.g.: [root@daemon /]# cobbler profile report --name="bla natty" No profile found: bla natty
Is this approach, in your opinion, still okay for buildiso? Shouldn't we ideally make this behaviour the same for all other commands too?
Thoughts?
Grtz, Jörgen Maas
On Sat, Aug 20, 2011 at 1:48 PM, Jörgen Maas jorgen.maas@gmail.com wrote:
On Sat, Aug 20, 2011 at 1:35 PM, James Cammarata jimi@sngx.net wrote:
<snip> > I would like to see this code use the input_string_or_list() function > from utils.py. It has always annoyed me that buildiso is the only > function that takes a list separated by commas instead of spaces.
Good suggestion, i will look into this and come up with a better patch. Thanks a lot for the review!
Grtz, Jörgen Maas
On Sun, Aug 21, 2011 at 4:10 AM, Jörgen Maas jorgen.maas@gmail.com wrote:
Hi James, others,
If i adjust the code to something like:
if systems is not None: # this check isn't really needed anymore since it's done in the utils function aswell which_systems = utils.input_string_or_list(systems) else: which_systems = []
The behaviour from the CLI will be like this: [root@daemon /]# cobbler buildiso --systems="x doosje"
Which is ok by me, but it isn't very consistent with other CLI commands, e.g.: [root@daemon /]# cobbler profile report --name="bla natty" No profile found: bla natty
Is this approach, in your opinion, still okay for buildiso? Shouldn't we ideally make this behaviour the same for all other commands too?
Thoughts?
Buildiso has always had some oddities, and I'm of the opinion it should be made to match the other CLI commands if it doesn't now. On the other hand, having an empty system list IS a valid state for buildiso, so I don't think the above should be considered an inconsistency.
Ok, i'll resend the updated patches again. Thanks
On Mon, Aug 22, 2011 at 4:56 AM, James Cammarata jimi@sngx.net wrote:
On Sun, Aug 21, 2011 at 4:10 AM, Jörgen Maas jorgen.maas@gmail.com wrote:
Hi James, others,
If i adjust the code to something like:
if systems is not None: # this check isn't really needed anymore since it's done in the utils function aswell which_systems = utils.input_string_or_list(systems) else: which_systems = []
The behaviour from the CLI will be like this: [root@daemon /]# cobbler buildiso --systems="x doosje"
Which is ok by me, but it isn't very consistent with other CLI commands, e.g.: [root@daemon /]# cobbler profile report --name="bla natty" No profile found: bla natty
Is this approach, in your opinion, still okay for buildiso? Shouldn't we ideally make this behaviour the same for all other commands too?
Thoughts?
Buildiso has always had some oddities, and I'm of the opinion it should be made to match the other CLI commands if it doesn't now. On the other hand, having an empty system list IS a valid state for buildiso, so I don't think the above should be considered an inconsistency. _______________________________________________ cobbler-devel mailing list cobbler-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler-devel
cobbler-devel@lists.fedorahosted.org