Alon Bar-Lev has posted comments on this change.

Change subject: ovirt-live: migrate ovirt live plugin to otopi
......................................................................


Patch Set 8:

(11 comments)

OK, just last styles issues to sync you up with our other projects :)

....................................................
File 
fedora/oVirtLiveFiles/root/usr/share/ovirt-engine/setup/plugins/ovirt-engine-setup/olive/core.py
Line 70: 
Line 71:     @plugin.event(
Line 72:         stage=plugin.Stages.STAGE_SETUP,
Line 73:         condition=lambda self: self.environment[
Line 74:             oliveconst.CoreEnv.ENABLE,
remove comma, as you add tuple as index which is wrong
Line 75:         ],
Line 76:     )
Line 77:     def _setup(self):
Line 78:         self._enabled = True


Line 80:     @plugin.event(
Line 81:         stage=plugin.Stages.CLOSEUP,
Line 82:         condition=lambda self: self._enabled,
Line 83:         before=(
Line 84:             oliveconst.Stages.OVIRTLIVE_CONFIG_STORAGE
comma or this won't be tuple
Line 85:         )
Line 86:     )
Line 87:     def _initapi(self):
Line 88:         self._engine_api = self._ovirtsdk_api.API(


Line 81:         stage=plugin.Stages.CLOSEUP,
Line 82:         condition=lambda self: self._enabled,
Line 83:         before=(
Line 84:             oliveconst.Stages.OVIRTLIVE_CONFIG_STORAGE
Line 85:         )
comma
Line 86:     )
Line 87:     def _initapi(self):
Line 88:         self._engine_api = self._ovirtsdk_api.API(
Line 89:             url='https://{fqdn}:{port}/api'.format(


Line 109:             osetupcons.Stages.AIO_CONFIG_VDSM,
Line 110:         ),
Line 111:     )
Line 112:     def _createstorage(self):
Line 113:         import pdb
why do you need this?
Line 114:         pdb.set_trace()
Line 115:         self.logger.debug('Attaching iso domain')
Line 116:         self._engine_api.datacenters.get(
Line 117:             self.environment[


Line 120:         ).storagedomains.add(
Line 121:             self._engine_api.storagedomains.get(
Line 122:                 self.environment[
Line 123:                     oliveconst.ConfigEnv.DEFAULT_ISO_NAME,
Line 124:                 ]
comma
Line 125:             )
Line 126:         )
Line 127: 
Line 128: 


Line 121:             self._engine_api.storagedomains.get(
Line 122:                 self.environment[
Line 123:                     oliveconst.ConfigEnv.DEFAULT_ISO_NAME,
Line 124:                 ]
Line 125:             )
comma
Line 126:         )
Line 127: 
Line 128: 
Line 129:     @plugin.event(


Line 143:             ],
Line 144:             self.environment[
Line 145:                 osetupcons.ConfigEnv.ISO_DOMAIN_SD_UUID
Line 146:             ],
Line 147:             "images",
'images',
Line 148:             osetupcons.ConfigEnv.ISO_DOMAIN_IMAGE_UID
Line 149:         )
Line 150:         for filename in fileList:
Line 151:             shutil.move(filename, targetPath)


Line 196: 
Line 197:         # Create NIC
Line 198:         self._engine_api.vms.get(
Line 199:             'local_vm'
Line 200:         ).nics.add(
almost sure that:

 self._engine_api.vms.get('local_vm').nics.add(

can be in one line :)
Line 201:             params.NIC(
Line 202:                 name='eth0',
Line 203:                 network=params.Network(name='ovirtmgmt'),
Line 204:                 interface='virtio',


Line 207: 
Line 208:         diskParam = params.Disk(
Line 209:             storage_domains=params.StorageDomains(
Line 210:                 storage_domain=[
Line 211:                     
self._engine_api.storagedomains.get('local_storage')
comma

please try to use tuple and see if it works
Line 212:                 ]
Line 213:             ),
Line 214:             size='6000MB',
Line 215:             type_='data',


Line 219:         )
Line 220: 
Line 221:         self._engine_api.vms.get(
Line 222:             'local_vm'
Line 223:         ).disks.add(
you can add(diskParm) no need to move to new line
Line 224:             diskParam
Line 225:         )
Line 226: 
Line 227: 


....................................................
File 
fedora/oVirtLiveFiles/root/usr/share/ovirt-engine/setup/plugins/ovirt-engine-setup/olive/oliveconst.py
Line 22: @util.export
Line 23: class Stages(object):
Line 24:     OVIRTLIVE_CONFIG_STORAGE = 'ovirtlivesetup.core.core.configstorage'
Line 25:     OVIRTLIVE_COPY_ISO = 'ovirtlivesetup.core.copy.iso'
Line 26:     OVIRTLIVE_CREATE_VM = 'ovirtlivesetup.core.create.vm'
you don' tneed the OVIRTLIVE_ prefix... the prefix is clear from the module.
Line 27: 
Line 28: 
Line 29: @util.export
Line 30: @util.codegen


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I703f64dc1183a6fe176d9d0352f93de381d906bb
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-live
Gerrit-Branch: master
Gerrit-Owner: Ohad Basan <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: David Caro <[email protected]>
Gerrit-Reviewer: Eyal Edri <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Lev Veyde <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Ohad Basan <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to