Change in vdsm[master]: introducing downloadFromStream verb

ybronhei at redhat.com ybronhei at redhat.com
Sun Jan 26 11:41:31 UTC 2014


Yaniv Bronhaim has posted comments on this change.

Change subject: introducing downloadFromStream verb
......................................................................


Patch Set 7:

(5 comments)

the -1 is mostly about the commit message. please upload your fixed version with your preferred code. no score from my side after that

http://gerrit.ovirt.org/#/c/23281/7/vdsm/BindingXMLRPC.py
File vdsm/BindingXMLRPC.py:

Line 144:             HEADER_VOLUME = 'X-Volume-Id'
Line 145:             HEADER_CONTENT_LENGTH = 'content-length'
Line 146:             HEADER_CONTENT_TYPE = 'content-type'
Line 147:             CONTENT_STREAM_TYPE = 'application/octet-stream'
Line 148: 
> there's no ned to move it to do_PUT and to have it defined everytime the me
you can also write it in global.py with no need. but as you don't use it anywhere else this will be redundant.. I'm just suggesting.
Line 149:             def setup(self):
Line 150:                 threadLocal.client = self.client_address[0]
Line 151:                 return basehandler.setup(self)
Line 152: 


http://gerrit.ovirt.org/#/c/23281/7/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 1679:                                 volUUID=None):
Line 1680:         """
Line 1681:         Download an image from a stream synchronously.
Line 1682: 
Line 1683:         Warning: Internal use only.
> internal use only means that theres no meaning to use it besides our use fr
ok.. doesn't mean anything to me. its public. but if you understand this label in the other methods, be my guess to put it also here
Line 1684:         """
Line 1685:         sdCache.produce(sdUUID)
Line 1686:         pool = self.getPool(spUUID)
Line 1687:         pool.downloadImage(methodArgs, sdUUID, imgUUID, volUUID)


http://gerrit.ovirt.org/#/c/23281/7/vdsm/storage/imageSharing.py
File vdsm/storage/imageSharing.py:

Line 28: 
Line 29: 
Line 30: log = logging.getLogger("Storage.ImageSharing")
Line 31: WAIT_TIMEOUT = 30
Line 32: BUFFER_SIZE = 65536
> what explanation would you suggest? those are arbitrary values that can alw
write it
Line 33: 
Line 34: 
Line 35: def httpGetSize(methodArgs):
Line 36:     headers = curlImgWrap.head(methodArgs.get('url'),


Line 53:     return size
Line 54: 
Line 55: 
Line 56: def streamGetSize(methodArgs):
Line 57:     return methodArgs['contentLength']
> that's internal implementation, and as i said - it's called from sp.py, tak
what is called from sp.py? whatever, if you insist to have a method that does nothing ..
Line 58: 
Line 59: 
Line 60: def httpDownloadImage(dstImgPath, methodArgs):
Line 61:     curlImgWrap.download(methodArgs.get('url'), dstImgPath,


Line 109:                       p.returncode, p.stderr.read(1000))
Line 110:             raise se.MiscFileWriteException()
Line 111: 
Line 112:     except Exception:
Line 113:         if p.returncode is None:
> see there
it does... my bad
Line 114:             p.kill()
Line 115:         raise
Line 116: 
Line 117: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I768b84799ed9fb2769c6d4240519d036f8988b99
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <laravot at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Liron Ar <laravot at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list