Wenyi Gao has uploaded a new change for review.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Fix multiple commands with same name in different vdsm-tool modules
Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Signed-off-by: Wenyi Gao wenyi@linux.vnet.ibm.com --- M vdsm-tool/vdsm-tool 1 file changed, 16 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/45/4645/1 -- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com
Adam Litke has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(1 inline comment)
Patch looks good. I am just suggesting a rewording of some text.
.................................................... File vdsm-tool/vdsm-tool Line 81: continue If you don't mind I will suggest a slight rewording of the text:
Warning: the command '%s' in module '%s' is being ignored because a command with the same name has already been registered by module '%s'
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Adam Litke has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 1: (1 inline comment)
.................................................... File vdsm-tool/vdsm-tool Line 93: If nothing is found, return None
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Wenyi Gao has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 1: (2 inline comments)
.................................................... File vdsm-tool/vdsm-tool Line 81: continue Good suggestion.
Line 93: Good point.
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com
Adam Litke has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 2: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com
Ryan Harper has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 2: I would prefer that you didn't submit this
(1 inline comment)
Clean up the typo, otherwise looks good.
.................................................... File vdsm-tool/vdsm-tool Line 75: ''%s' is bing ignored, because a command ' s/bing/being
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com
Federico Simoncelli has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(4 inline comments)
.................................................... File vdsm-tool/vdsm-tool Line 74: sys.stdout.write('\nWarning: the command '%s' in module ' The format of the string is confusing for me. We could define an error message (at the moment just as a global variable in this file, we'll do it in a fancier way later), and use it here. It's also better for localization (in the future).
ERROR_SAME_COMMAND = """\ Warning: the command '%s' in module '%s' is being ignored, because a command \ with the same name has already been registered by module '%s'.\ """
And then here you can use:
print textwrap.fill(ERROR_SAME_COMMAND % (cmd_name, mod_name, tool_command[cmd_name]["module"].__name__))
Line 81: tool_command[cmd_name] = cmd Let's cache the module name here:
tool_command[cmd_name] = {"module": module, "command": cmd}
(You need to modify all the parts that are using tool_command)
Line 87: def _get_module_name(command): We can get rid of this since we cache the module in load_modules.
Line 140: tool_command[cmd](*args[1:]) Here:
tool_command[cmd]["command"](*args[1:])
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com
Federico Simoncelli has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 4: Looks good to me, but someone else must approve
Did you test it?
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com
Ryan Harper has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 4: (1 inline comment)
.................................................... File vdsm-tool/vdsm-tool Line 33: Warning: the command '%s' in module '%s' is being ignored, because a command \ nit: remove the comma after ignored
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com
Wenyi Gao has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 5: Verified
I tested and it works okey.
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com
Mark Wu has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 5: Verified; Looks good to me, but someone else must approve
I copy one vdsm-tool script to a new a one. I can see the warning message added in this patch
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com
ShaoHe Feng has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 5: Looks good to me, but someone else must approve
(1 inline comment)
.................................................... File vdsm-tool/vdsm-tool Line 35: """ triple quotes and single quote are same here. But I still understand the difference between Triple quotes and single quote.
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Patch Set 5: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Fix multiple commands with same name in different vdsm-tool modules ......................................................................
Fix multiple commands with same name in different vdsm-tool modules
Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Signed-off-by: Wenyi Gao wenyi@linux.vnet.ibm.com --- M vdsm-tool/vdsm-tool 1 file changed, 13 insertions(+), 3 deletions(-)
Approvals: ShaoHe Feng: Looks good to me, but someone else must approve Mark Wu: Verified; Looks good to me, but someone else must approve Wenyi Gao: Verified Dan Kenigsberg: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/4645 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: I6a289a202e0efcb7f2948b6200ea39552164f5b6 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com
vdsm-patches@lists.fedorahosted.org