I have extended the prototype i'm working on for the system editing. Attached is a new patch that implements the following things:
Buttons above the system list Support Add/Rename/Delete operations from the system list Support changing netboot/profile on multiple systems Support power control on multiple systems (requires remote power_api patch)
Comments are welcome.
Regards Peter
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
Vreman, Peter - Acision wrote:
I have extended the prototype i’m working on for the system editing. Attached is a new patch that implements the following things:
Buttons above the system list
Support Add/Rename/Delete operations from the system list
Support changing netboot/profile on multiple systems
Support power control on multiple systems (requires remote power_api patch)
Comments are welcome.
Regards
Peter
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
cobbler mailing list cobbler@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler
Thanks for submitting this.
I've noticed a problem with this patch in that it appears to introduce some inconsistent Python tabbing. While I /love/ Python for it's whitespace significance, mixing spaces and tabs can lead to problems with the interpreter, so this is something that must be watched for. For instance, when I'm applying this, I see the warning:
[mdehaan@localhost cobbler]$ git apply /tmp/system_actions.diff /tmp/system_actions.diff:23: space before tab in indent.
In this case the patch applies as:
def system_delete(self,targetlist=None,**args): if not self.__xmlrpc_setup(): return self.xmlrpc_auth_failure() if targetlist is None: return self.error_page("Targetlist parameter is REQUIRED.")
As you can see, the "if targetlist is None" code can never be executed because it immediately follows the return.
I'd also just modify the system_delete method to require the targetlist because it seems to be a code error if it's not supplied, otherwise the user won't understand what a "targetlist" is.
If the system delete code works out well, I take it you're going to make all the other object deletes work the same way?
This seems like it would be very nice for batch deletes.
Thanks!
--Michael
Michael DeHaan wrote:
Vreman, Peter - Acision wrote:
I have extended the prototype i’m working on for the system editing. Attached is a new patch that implements the following things:
Buttons above the system list
Support Add/Rename/Delete operations from the system list
Support changing netboot/profile on multiple systems
Support power control on multiple systems (requires remote power_api patch)
Comments are welcome.
Regards
Peter
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
cobbler mailing list cobbler@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler
Thanks for submitting this.
I've noticed a problem with this patch in that it appears to introduce some inconsistent Python tabbing. While I /love/ Python for it's whitespace significance, mixing spaces and tabs can lead to problems with the interpreter, so this is something that must be watched for. For instance, when I'm applying this, I see the warning:
[mdehaan@localhost cobbler]$ git apply /tmp/system_actions.diff /tmp/system_actions.diff:23: space before tab in indent.
In this case the patch applies as:
def system_delete(self,targetlist=None,**args): if not self.__xmlrpc_setup(): return self.xmlrpc_auth_failure() if targetlist is None: return self.error_page("Targetlist parameter is REQUIRED.")
Ironically, my mailreader keeps the above from displaying correctly. What I meant to say was the second to list line is in line with the return.
As you can see, the "if targetlist is None" code can never be executed because it immediately follows the return.
I'd also just modify the system_delete method to require the targetlist because it seems to be a code error if it's not supplied, otherwise the user won't understand what a "targetlist" is.
If the system delete code works out well, I take it you're going to make all the other object deletes work the same way?
This seems like it would be very nice for batch deletes.
Thanks!
--Michael
Thanks for submitting this.
I've noticed a problem with this patch in that it appears to introduce some inconsistent Python tabbing. While I /love/ Python for it's whitespace significance, mixing spaces and tabs can lead to problems with the interpreter, so this is something that must be watched for. For instance, when I'm applying this, I see the warning:
[mdehaan@localhost cobbler]$ git apply /tmp/system_actions.diff /tmp/system_actions.diff:23: space before tab in indent.
In this case the patch applies as:
def system_delete(self,targetlist=None,**args): if not self.__xmlrpc_setup(): return self.xmlrpc_auth_failure() if targetlist is None: return self.error_page("Targetlist parameter is REQUIRED.")
As you can see, the "if targetlist is None" code can never be executed because it immediately follows the return.
I'd also just modify the system_delete method to require the targetlist because it seems to be a code error if it's not supplied, otherwise the user won't understand what a "targetlist" is.
If the system delete code works out well, I take it you're going to make all the other object deletes work the same way?
This seems like it would be very nice for batch deletes.
The batch operations functionality is the main reason why I created the patch.
The patch can be extended to include all other types profiles/distros/images the same way. But before to continue with the other types it is better to agree on how the actions will be presented to the user. I quickly made a hack with the buttons because it was easy. But there might be other ideas for the presentation.
Shall I submit a new patch with the spaces fixed?
Regards Peter
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
Vreman, Peter - Acision wrote:
Thanks for submitting this.
I've noticed a problem with this patch in that it appears to introduce some inconsistent Python tabbing. While I /love/ Python for it's whitespace significance, mixing spaces and tabs can lead to problems with the interpreter, so this is something that must be watched for. For instance, when I'm applying this, I see the warning:
[mdehaan@localhost cobbler]$ git apply /tmp/system_actions.diff /tmp/system_actions.diff:23: space before tab in indent.
In this case the patch applies as:
def system_delete(self,targetlist=None,**args): if not self.__xmlrpc_setup(): return self.xmlrpc_auth_failure() if targetlist is None: return self.error_page("Targetlist parameter is REQUIRED.")
As you can see, the "if targetlist is None" code can never be executed because it immediately follows the return.
I'd also just modify the system_delete method to require the targetlist because it seems to be a code error if it's not supplied, otherwise the user won't understand what a "targetlist" is.
If the system delete code works out well, I take it you're going to make all the other object deletes work the same way?
This seems like it would be very nice for batch deletes.
The batch operations functionality is the main reason why I created the patch.
The patch can be extended to include all other types profiles/distros/images the same way. But before to continue with the other types it is better to agree on how the actions will be presented to the user. I quickly made a hack with the buttons because it was easy. But there might be other ideas for the presentation.
It depends how the buttons look. Image buttons are ok, just doing anchor tags with javascript actions also preserves space. I assume we're talking about a row of checkboxes and an "apply" at the bottom, rather than one button for each system that does all the calls when it is clicked?
Shall I submit a new patch with the spaces fixed?
Yes, please do. Thanks!
Regards Peter
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
cobbler mailing list cobbler@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler
If the system delete code works out well, I take it you're going to
make
all the other object deletes work the same way?
This seems like it would be very nice for batch deletes.
The batch operations functionality is the main reason why I created the
patch.
The patch can be extended to include all other types
profiles/distros/images the same way. But before to continue with the other types it is better to agree on how the actions will be presented to the user. I quickly made a hack with the buttons because it was easy. But there might be other ideas for the presentation.
It depends how the buttons look. Image buttons are ok, just doing anchor tags with javascript actions also preserves space. I assume we're talking about a row of checkboxes and an "apply" at the bottom, rather than one button for each system that does all the calls when it is clicked?
Shall I submit a new patch with the spaces fixed?
Yes, please do. Thanks!
Attached.
Peter
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
Vreman, Peter - Acision wrote:
If the system delete code works out well, I take it you're going to
make
all the other object deletes work the same way?
This seems like it would be very nice for batch deletes.
The batch operations functionality is the main reason why I created the
patch.
The patch can be extended to include all other types
profiles/distros/images the same way. But before to continue with the other types it is better to agree on how the actions will be presented to the user. I quickly made a hack with the buttons because it was easy. But there might be other ideas for the presentation.
It depends how the buttons look. Image buttons are ok, just doing anchor tags with javascript actions also preserves space. I assume we're talking about a row of checkboxes and an "apply" at the bottom, rather than one button for each system that does all the calls when it is clicked?
Shall I submit a new patch with the spaces fixed?
Yes, please do. Thanks!
Attached.
Peter
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
cobbler mailing list cobbler@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler
Hey Peter,
Ok, looks like a very good start and /very/ useful. I have a few suggestions that could make this a bit more usable, I think, and they are all just presentation related.
Currently there's a row of buttons on the top of the page. Some depend on what is selected, some allow multi selections, some do not. I think that is a bit confusing, or it was to me at least.
What I would recommend is that: -- The "Add" feature only be available from the left hand side sidebar as it was before after your first couple of patches. -- An action like "Copy" or "Rename" that can only take one at a time be shown on each line, with each system record. Let's do the same with edit, so you have on each line: -- For multi-select options, move those buttons to the bottom of the list and make some header called "batch actions" that shows -- Remove the "edit" link that surrounds the system name and move it to a button
If I may show this in really bad ASCII diagrams:
| System: | Edit Button | Template: | View Rendered: | Copy | Rename checkbox | systemname1 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname2 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname3 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname4 | edit | kickstart template | rendered kickstart | copy button | rename button |
(empty space)
batch actions on checked systems: power control | netboot status | delete
Basically that would give it just a little bit better idea of whether clicking a button would do that particular action to the particular system or would be a batch action.
Also if we do that we can remove the "Edit/Rename" logic from the system_edit pages along with the delete buttons on those pages.
I would definitely be interested in seeing that.
Thanks!
--Michael
Michael DeHaan wrote:
Vreman, Peter - Acision wrote:
If the system delete code works out well, I take it you're going to
make
all the other object deletes work the same way?
This seems like it would be very nice for batch deletes.
The batch operations functionality is the main reason why I created the
patch.
The patch can be extended to include all other types
profiles/distros/images the same way. But before to continue with the other types it is better to agree on how the actions will be presented to the user. I quickly made a hack with the buttons because it was easy. But there might be other ideas for the presentation.
It depends how the buttons look. Image buttons are ok, just doing anchor tags with javascript actions also preserves space. I assume we're talking about a row of checkboxes and an "apply" at the bottom, rather than one button for each system that does all the calls when it is clicked?
Shall I submit a new patch with the spaces fixed?
Yes, please do. Thanks!
Attached.
Peter
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
cobbler mailing list cobbler@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler
Hey Peter,
Ok, looks like a very good start and /very/ useful. I have a few suggestions that could make this a bit more usable, I think, and they are all just presentation related.
Currently there's a row of buttons on the top of the page. Some depend on what is selected, some allow multi selections, some do not. I think that is a bit confusing, or it was to me at least.
What I would recommend is that: -- The "Add" feature only be available from the left hand side sidebar as it was before after your first couple of patches. -- An action like "Copy" or "Rename" that can only take one at a time be shown on each line, with each system record. Let's do the same with edit, so you have on each line: -- For multi-select options, move those buttons to the bottom of the list and make some header called "batch actions" that shows -- Remove the "edit" link that surrounds the system name and move it to a button
If I may show this in really bad ASCII diagrams:
| System: | Edit Button | Template:
| View Rendered: | Copy | Rename checkbox | systemname1 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname2 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname3 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname4 | edit | kickstart template | rendered kickstart | copy button | rename button |
Correction, remove "kickstart template" from the above. per-system-templates are a fringe feature and don't need to show up there.
(empty space)
batch actions on checked systems: power control | netboot status | delete
Basically that would give it just a little bit better idea of whether clicking a button would do that particular action to the particular system or would be a batch action.
Also if we do that we can remove the "Edit/Rename" logic from the system_edit pages along with the delete buttons on those pages.
I would definitely be interested in seeing that.
Thanks!
--Michael
cobbler mailing list cobbler@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler
Ok, looks like a very good start and /very/ useful. I have a few suggestions that could make this a bit more usable, I think, and they are all just presentation related.
Currently there's a row of buttons on the top of the page. Some depend on what is selected, some allow multi selections, some do not. I think that is a bit confusing, or it was to me at least.
What I would recommend is that: -- The "Add" feature only be available from the left hand side sidebar as it was before after your first couple of patches. -- An action like "Copy" or "Rename" that can only take one at a time be shown on each line, with each system record. Let's do the same with edit, so you have on each line: -- For multi-select options, move those buttons to the bottom of the list and make some header called "batch actions" that shows -- Remove the "edit" link that surrounds the system name and move it to a button
If I may show this in really bad ASCII diagrams:
| System: | Edit Button | Template:
| View Rendered: | Copy | Rename checkbox | systemname1 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname2 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname3 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname4 | edit | kickstart template | rendered kickstart | copy button | rename button |
Correction, remove "kickstart template" from the above. per-system-templates are a fringe feature and don't need to show up there.
(empty space)
batch actions on checked systems: power control | netboot status | delete
Basically that would give it just a little bit better idea of whether clicking a button would do that particular action to the particular system or would be a batch action.
Also if we do that we can remove the "Edit/Rename" logic from the system_edit pages along with the delete buttons on those pages.
I would definitely be interested in seeing that.
Thanks for the feedback. I'll have a look at it.
Peter
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
Ok, looks like a very good start and /very/ useful. I have a few suggestions that could make this a bit more usable, I think, and they are all just presentation related.
Currently there's a row of buttons on the top of the page. Some
depend
on what is selected, some allow multi selections, some do not. I
think
that is a bit confusing, or it was to me at least.
What I would recommend is that: -- The "Add" feature only be available from the left hand side sidebar as it was before after your first couple of patches. -- An action like "Copy" or "Rename" that can only take one at a time
be
shown on each line, with each system record. Let's do the same with edit, so you have on each line: -- For multi-select options, move those buttons to the bottom of the list and make some header called "batch actions" that shows -- Remove the "edit" link that surrounds the system name and move it
to
a button
If I may show this in really bad ASCII diagrams:
| System: | Edit Button | Template:
| View Rendered: | Copy | Rename checkbox | systemname1 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname2 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname3 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname4 | edit | kickstart template | rendered kickstart | copy button | rename button |
Correction, remove "kickstart template" from the above. per-system-templates are a fringe feature and don't need to show up
there.
(empty space)
batch actions on checked systems: power control | netboot status | delete
Basically that would give it just a little bit better idea of whether clicking a button would do that particular action to the particular system or would be a batch action.
Also if we do that we can remove the "Edit/Rename" logic from the system_edit pages along with the delete buttons on those pages.
I would definitely be interested in seeing that.
Thanks for the feedback. I'll have a look at it.
I have updated the patch with the provided feedback. Also the system edit page has been updated to remove the rename,copy and delete operations.
Regards, Peter
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
Vreman, Peter - Acision wrote:
Ok, looks like a very good start and /very/ useful. I have a few suggestions that could make this a bit more usable, I think, and they are all just presentation related.
Currently there's a row of buttons on the top of the page. Some
depend
on what is selected, some allow multi selections, some do not. I
think
that is a bit confusing, or it was to me at least.
What I would recommend is that: -- The "Add" feature only be available from the left hand side sidebar as it was before after your first couple of patches. -- An action like "Copy" or "Rename" that can only take one at a time
be
shown on each line, with each system record. Let's do the same with edit, so you have on each line: -- For multi-select options, move those buttons to the bottom of the list and make some header called "batch actions" that shows -- Remove the "edit" link that surrounds the system name and move it
to
a button
If I may show this in really bad ASCII diagrams:
| System: | Edit Button | Template:
| View Rendered: | Copy | Rename checkbox | systemname1 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname2 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname3 | edit | kickstart template | rendered kickstart | copy button | rename button | checkbox | systemname4 | edit | kickstart template | rendered kickstart | copy button | rename button |
Correction, remove "kickstart template" from the above. per-system-templates are a fringe feature and don't need to show up
there.
(empty space)
batch actions on checked systems: power control | netboot status | delete
Basically that would give it just a little bit better idea of whether clicking a button would do that particular action to the particular system or would be a batch action.
Also if we do that we can remove the "Edit/Rename" logic from the system_edit pages along with the delete buttons on those pages.
I would definitely be interested in seeing that.
Thanks for the feedback. I'll have a look at it.
I have updated the patch with the provided feedback. Also the system edit page has been updated to remove the rename,copy and delete operations.
Regards, Peter
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
cobbler mailing list cobbler@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler
Thanks Peter,
I'll check this out soon -- testing 1.4 takes importance so we may be looking to get this in for 1.5.
--Michael
I have updated the patch with the provided feedback. Also the system edit page has been updated to remove the rename,copy and delete operations.
Regards, Peter
Peter,
Wow. This is /fantastic/ stuff. I'm impressed -- it looks great and delivers some very useful batch editing functionality to the systems page.
I've applied this to the devel branch (1.5/1.6).
Everything I've tested seems to work great.
Thanks!!!
--Michael
This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.
cobbler mailing list cobbler@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler
cobbler@lists.fedorahosted.org