Change in vdsm[master]: storagePool: Reject unknown method in uploadVolume

Adam Litke has uploaded a new change for review.

Change subject: storagePool: Reject unknown method in uploadVolume
......................................................................

storagePool: Reject unknown method in uploadVolume

Address a previously identified TODO in the code and report an error if a method
other than 'wget' or 'rsync' is specified.

Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Signed-off-by: Adam Litke 
---
M vdsm/storage/sp.py
1 file changed, 4 insertions(+), 2 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/671/1
--
To view, visit http://gerrit.ovirt.org/671
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke

Adam Litke has posted comments on this change.

Change subject: storagePool: Reject unknown method in uploadVolume
......................................................................

Patch Set 1: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/671
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: storagePool: Reject unknown method in uploadVolume
......................................................................

Patch Set 1: Looks good to me, but someone else must approve

(1 inline comment)

heh, thanks for fixing ancient historical errors.

....................................................
File vdsm/storage/sp.py
Line 1980:                 self.log.error("uploadVolume - method '%s' not supported" % method)
minor: Saggi has something against % in logging messages, and I have something against long lines

--
To view, visit http://gerrit.ovirt.org/671
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg

Saggi Mizrahi has posted comments on this change.

Change subject: storagePool: Reject unknown method in uploadVolume
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/storage/sp.py
Line 1980:                 self.log.error("uploadVolume - method '%s' not supported" % method)
My complaint is not the same as your pettiness.

When letting the logging infra to do the formatting you actually avoid formatting if that log level is disabled. If you format outside logging you still pay the performance price for the string formatting even if the line will never be printed.

This is, in fact, the reason why all the logging methods have their own formatting ability.

--
To view, visit http://gerrit.ovirt.org/671
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Saggi Mizrahi has posted comments on this change.

Change subject: storagePool: Reject unknown method in uploadVolume
......................................................................

Patch Set 1:

Also you shouldn't +1 yourself.
You could -1 yourself if you found an error in the posted version and you don't want anyone mistakenly pushing this with the bug.

But you should never +1 yourself. That's just pure arrogance ;)

--
To view, visit http://gerrit.ovirt.org/671
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Adam Litke has posted comments on this change.

Change subject: storagePool: Reject unknown method in uploadVolume
......................................................................

Patch Set 1: No score

(1 inline comment)

....................................................
File vdsm/storage/sp.py
Line 1980:                 self.log.error("uploadVolume - method '%s' not supported" % method)
So if I change this to :

self.log.error("uploadVolume - method '%s' not supported", method)

would that work?

--
To view, visit http://gerrit.ovirt.org/671
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: storagePool: Reject unknown method in uploadVolume
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/storage/sp.py
Line 1980:                 self.log.error("uploadVolume - method '%s' not supported" % method)
yes, it should.

--
To view, visit http://gerrit.ovirt.org/671
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Saggi Mizrahi has posted comments on this change.

Change subject: storagePool: Reject unknown method in uploadVolume
......................................................................

Patch Set 2: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/671
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: storagePool: Reject unknown method in uploadVolume
......................................................................

Patch Set 2: Verified; Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/671
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: storagePool: Reject unknown method in uploadVolume
......................................................................

storagePool: Reject unknown method in uploadVolume

Address a previously identified TODO in the code and report an error if a method
other than 'wget' or 'rsync' is specified.

Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Signed-off-by: Adam Litke 
---
M vdsm/storage/sp.py
1 file changed, 4 insertions(+), 2 deletions(-)

Approvals:
  Saggi Mizrahi: Looks good to me, but someone else must approve
  Dan Kenigsberg: Verified; Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/671
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I35267fab74a98bab351e4e66b9349ded69c573ee
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi