Nir Soffer has posted comments on this change. Change subject: sdm: Add create_volume job ......................................................................
Patch Set 9: Code-Review-1 (3 comments) Nice https://gerrit.ovirt.org/#/c/50221/9/vdsm/storage/sdm/api/Makefile.am File vdsm/storage/sdm/api/Makefile.am: Line 24: Line 25: dist_vdsmsdmapi_PYTHON = \ Line 26: __init__.py \ Line 27: create_volume.py \ Line 28: sdm_job.py \ sdm_job.Job looks like the base class for all sdm.api.xxxx.Job, so maybe call it base.py: # sdm/api/delete_image.py from . import base class Job(base.Job): ... https://gerrit.ovirt.org/#/c/50221/9/vdsm/storage/sdm/api/sdm_job.py File vdsm/storage/sdm/api/sdm_job.py: Line 37: self._status = jobs.STATUS.PENDING Line 38: self.host_id = host_id Line 39: Line 40: def run(self): Line 41: vars.job_id = self.id You missed my comments from previous versions: - Move vars.job_id = self.id just before the try: - Add finally: to clean it up Line 42: self._status = jobs.STATUS.RUNNING Line 43: try: Line 44: self._run() Line 45: except se.StorageException as e: Line 42: self._status = jobs.STATUS.RUNNING Line 43: try: Line 44: self._run() Line 45: except se.StorageException as e: Line 46: self.log.exception("Job (id:%s desc:%s) failed", Use (id=xxx desc=yyy) Line 47: self.id, self.description) Line 48: self._status = jobs.STATUS.FAILED Line 49: self._error = e Line 50: except Exception as e: -- To view, visit https://gerrit.ovirt.org/50221 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia614059f52c9625da7841ea9fbca2b2f2375cd75 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
