Adam Litke has uploaded a new change for review.
Change subject: jobs: Move _run from sdm.base.Job to jobs.Job
......................................................................
jobs: Move _run from sdm.base.Job to jobs.Job
The _run implementation in sdm.base.Job is generally useful. It performs state
checking and error handling that can be applicable to all job types. Move it
to job.Job. The tests associated with this functionality need to move to
jobsTests.py which means we no longer need storage_sdm_api_test.py or
sdmtestlib.py
Change-Id: I194eb154d70e846b7ec5d462faa26b3aa35d2ff6
Signed-off-by: Adam Litke <ali...@redhat.com>
---
M lib/vdsm/jobs.py
M tests/Makefile.am
M tests/jobsTests.py
D tests/sdmtestlib.py
D tests/storage_sdm_api_test.py
M tests/storage_sdm_copy_data_test.py
M tests/storage_sdm_create_volume_test.py
M tests/testlib.py
M vdsm/storage/sdm/api/base.py
9 files changed, 73 insertions(+), 127 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/01/62001/1
diff --git a/lib/vdsm/jobs.py b/lib/vdsm/jobs.py
index 2669c85..21b6cb0 100644
--- a/lib/vdsm/jobs.py
+++ b/lib/vdsm/jobs.py
@@ -22,7 +22,10 @@
import logging
import threading
+from vdsm import exception
from vdsm import response
+
+from vdsm.storage.threadlocal import vars
_lock = threading.Lock()
@@ -119,12 +122,35 @@
logging.info('Job %r aborting...', self._id)
self._abort()
+ def run(self):
+ self._status = STATUS.RUNNING
+ vars.job_id = self.id
+ try:
+ self._run()
+ except Exception as e:
+ logging.exception("Job (id=%s desc=%s) failed",
+ self.id, self.description)
+ if not isinstance(e, exception.VdsmException):
+ e = exception.GeneralException(str(e))
+ self._error = e
+ self._status = STATUS.FAILED
+ else:
+ self._status = STATUS.DONE
+ finally:
+ vars.job_id = None
+
def _abort(self):
"""
May be implemented by child class
"""
raise AbortNotSupported()
+ def _run(self):
+ """
+ Must be implemented by child class
+ """
+ raise NotImplementedError()
+
def __repr__(self):
s = "<{self.__class__.__name__} id={self.id} status={self.status} "
if self.progress is not None:
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 467f2d4..f58a319 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -125,7 +125,6 @@
storage_hsm_test.py \
storage_monitor_test.py \
storage_rwlock_test.py \
- storage_sdm_api_test.py \
storage_sdm_copy_data_test.py \
storage_sdm_create_volume_test.py \
storage_volume_artifacts_test.py \
@@ -226,7 +225,6 @@
storageServerTests.py \
storage_rwlock_test.py \
storage_blkdiscard_test.py \
- storage_sdm_api_test.py \
storage_sdm_create_volume_test.py \
storage_sdm_copy_data_test.py \
storage_volume_artifacts_test.py \
@@ -338,7 +336,6 @@
fake-ssh-add \
fake-ssh-agent \
monkeypatch.py \
- sdmtestlib.py \
storagefakelib.py \
storagetestlib.py \
testlib.py \
diff --git a/tests/jobsTests.py b/tests/jobsTests.py
index bc5baef..dce6535 100644
--- a/tests/jobsTests.py
+++ b/tests/jobsTests.py
@@ -20,20 +20,29 @@
import uuid
from vdsm import exception, jobs, response, schedule
+from vdsm.storage.threadlocal import vars
from testlib import VdsmTestCase, expandPermutations, permutations
+from testlib import wait_for_job
class TestingJob(jobs.Job):
_JOB_TYPE = 'testing'
- def __init__(self, status=jobs.STATUS.PENDING):
+ def __init__(self, status=jobs.STATUS.PENDING, exception=None):
jobs.Job.__init__(self, str(uuid.uuid4()))
self._aborted = False
self._status = status
+ self._exception = exception
def _abort(self):
self._aborted = True
+
+ def _run(self):
+ assert(self.status == jobs.STATUS.RUNNING)
+ assert(vars.job_id == self.id)
+ if self._exception:
+ raise self._exception
class FooJob(TestingJob):
@@ -69,6 +78,13 @@
def tearDown(self):
jobs.stop()
+
+ def run_job(self, job):
+ self.assertEqual(jobs.STATUS.PENDING, job.status)
+ self.assertIsNone(getattr(vars, 'job_id', None))
+ job.run()
+ wait_for_job(job)
+ self.assertIsNone(getattr(vars, 'job_id', None))
def test_job_initial_state(self):
job = TestingJob()
@@ -231,3 +247,22 @@
job.progress = 32
rep = repr(job)
self.assertIn("progress=32%", rep)
+
+ def test_running_states(self):
+ job = TestingJob()
+ self.run_job(job)
+ self.assertEqual(jobs.STATUS.DONE, job.status)
+
+ def test_default_exception(self):
+ message = "testing failure"
+ job = TestingJob(exception=Exception(message))
+ self.run_job(job)
+ self.assertEqual(jobs.STATUS.FAILED, job.status)
+ self.assertIsInstance(job.error, exception.GeneralException)
+ self.assertIn(message, str(job.error))
+
+ def test_vdsm_exception(self):
+ job = TestingJob(exception=exception.VdsmException())
+ self.run_job(job)
+ self.assertEqual(jobs.STATUS.FAILED, job.status)
+ self.assertIsInstance(job.error, exception.VdsmException)
diff --git a/tests/sdmtestlib.py b/tests/sdmtestlib.py
deleted file mode 100644
index 8f95218..0000000
--- a/tests/sdmtestlib.py
+++ /dev/null
@@ -1,25 +0,0 @@
-# Copyright 2015 Red Hat, Inc.
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-#
-# Refer to the README and COPYING files for full details of the license
-#
-
-import time
-
-
-def wait_for_job(job):
- while job.active:
- time.sleep(1)
diff --git a/tests/storage_sdm_api_test.py b/tests/storage_sdm_api_test.py
deleted file mode 100644
index c8d93d2..0000000
--- a/tests/storage_sdm_api_test.py
+++ /dev/null
@@ -1,73 +0,0 @@
-#
-# Copyright 2016 Red Hat, Inc.
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-#
-# Refer to the README and COPYING files for full details of the license
-#
-
-import uuid
-
-from testlib import VdsmTestCase
-from sdmtestlib import wait_for_job
-
-from vdsm import jobs
-from vdsm import exception
-from vdsm.storage.threadlocal import vars
-
-from storage.sdm.api import base
-
-
-class ApiBaseTests(VdsmTestCase):
-
- def run_job(self, job):
- self.assertEqual(jobs.STATUS.PENDING, job.status)
- self.assertIsNone(getattr(vars, 'job_id', None))
- job.run()
- wait_for_job(job)
- self.assertIsNone(getattr(vars, 'job_id', None))
-
- def test_states(self):
- job = TestingJob()
- self.run_job(job)
- self.assertEqual(jobs.STATUS.DONE, job.status)
-
- def test_default_exception(self):
- message = "testing failure"
- job = TestingJob(Exception(message))
- self.run_job(job)
- self.assertEqual(jobs.STATUS.FAILED, job.status)
- self.assertIsInstance(job.error, exception.GeneralException)
- self.assertIn(message, str(job.error))
-
- def test_vdsm_exception(self):
- job = TestingJob(exception.VdsmException())
- self.run_job(job)
- self.assertEqual(jobs.STATUS.FAILED, job.status)
- self.assertIsInstance(job.error, exception.VdsmException)
-
-
-class TestingJob(base.Job):
-
- def __init__(self, exception=None):
- job_id = str(uuid.uuid4())
- super(TestingJob, self).__init__(job_id, 'testing_job', 'host_id')
- self.exception = exception
-
- def _run(self):
- assert(self.status == jobs.STATUS.RUNNING)
- assert(vars.job_id == self.id)
- if self.exception:
- raise self.exception
diff --git a/tests/storage_sdm_copy_data_test.py
b/tests/storage_sdm_copy_data_test.py
index def8934..a92fa2c 100644
--- a/tests/storage_sdm_copy_data_test.py
+++ b/tests/storage_sdm_copy_data_test.py
@@ -28,7 +28,7 @@
from storagetestlib import make_qemu_chain, write_qemu_chain, verify_qemu_chain
from storagetestlib import ChainVerificationError
from testlib import VdsmTestCase, expandPermutations, permutations
-from sdmtestlib import wait_for_job
+from testlib import wait_for_job
from vdsm import jobs
from vdsm import qemuimg
diff --git a/tests/storage_sdm_create_volume_test.py
b/tests/storage_sdm_create_volume_test.py
index 92f5e9f..ec6b065 100644
--- a/tests/storage_sdm_create_volume_test.py
+++ b/tests/storage_sdm_create_volume_test.py
@@ -25,7 +25,7 @@
from monkeypatch import MonkeyPatchScope
from storagefakelib import FakeResourceManager
from testlib import VdsmTestCase, recorded, expandPermutations, permutations
-from sdmtestlib import wait_for_job
+from testlib import wait_for_job
from vdsm import exception
from vdsm import jobs
diff --git a/tests/testlib.py b/tests/testlib.py
index aa44737..1f0cef6 100644
--- a/tests/testlib.py
+++ b/tests/testlib.py
@@ -35,6 +35,7 @@
from six.moves import range
import tempfile
import threading
+import time
from contextlib import contextmanager
import xml.etree.ElementTree as ET
@@ -577,3 +578,11 @@
path = os.path.join(dir_name, 'data', filename)
with open(path) as src:
return src.read()
+
+
+def wait_for_job(job):
+ """
+ Wait for a jobs.Job to complete (either success or failure)
+ """
+ while job.active:
+ time.sleep(1)
diff --git a/vdsm/storage/sdm/api/base.py b/vdsm/storage/sdm/api/base.py
index a342300..760f050 100644
--- a/vdsm/storage/sdm/api/base.py
+++ b/vdsm/storage/sdm/api/base.py
@@ -20,35 +20,12 @@
from __future__ import absolute_import
-import logging
-
from vdsm import jobs
-from vdsm import exception
-from vdsm.storage.threadlocal import vars
class Job(jobs.Job):
_JOB_TYPE = "storage"
- log = logging.getLogger('storage.sdmjob')
def __init__(self, job_id, desc, host_id):
super(Job, self).__init__(job_id, desc)
- self._status = jobs.STATUS.PENDING
self.host_id = host_id
-
- def run(self):
- self._status = jobs.STATUS.RUNNING
- vars.job_id = self.id
- try:
- self._run()
- except Exception as e:
- self.log.exception("Job (id=%s desc=%s) failed",
- self.id, self.description)
- if not isinstance(e, exception.VdsmException):
- e = exception.GeneralException(str(e))
- self._error = e
- self._status = jobs.STATUS.FAILED
- else:
- self._status = jobs.STATUS.DONE
- finally:
- vars.job_id = None
--
To view, visit https://gerrit.ovirt.org/62001
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I194eb154d70e846b7ec5d462faa26b3aa35d2ff6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <ali...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
