Piotr Kliczewski has uploaded a new change for review.
Change subject: yml: parameter type fixes for StoragePool.connect ......................................................................
yml: parameter type fixes for StoragePool.connect
Change-Id: I19b6f25c17e697702ec61eba6b11f256c1df4d83 Signed-off-by: Piotr Kliczewski piotr.kliczewski@gmail.com --- M lib/api/vdsm-api.yml M tests/vdsmapi_test.py 2 files changed, 18 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/59702/1
diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml index 5eb091c..6cb6c9a 100644 --- a/lib/api/vdsm-api.yml +++ b/lib/api/vdsm-api.yml @@ -5599,11 +5599,11 @@ name: StorageDomainStatus type: enum values: - Active: The domain is attached to a Storage Pool and is activated - Attached: The domain is attached to a Storage Pool but is + active: The domain is attached to a Storage Pool and is activated + attached: The domain is attached to a Storage Pool but is deactivated - Unattached: The domain is not attached to a Storage Pool - Unknown: The status of the Storage Domain is not known + unattached: The domain is not attached to a Storage Pool + unknown: The status of the Storage Domain is not known
StorageDomainStatusMap: &StorageDomainStatusMap added: '3.1' diff --git a/tests/vdsmapi_test.py b/tests/vdsmapi_test.py index 38f5449..fac3bf4 100644 --- a/tests/vdsmapi_test.py +++ b/tests/vdsmapi_test.py @@ -502,3 +502,17 @@ _schema.schema().verify_args( vdsmapi.MethodRep('StoragePool', 'connectStorageServer'), params) + + def test_sp_connect(self): + params = {u'masterVersion': 33, + u'domainDict': + {u'0900c0cd-8422-497a-85c5-1bb34b2b6b65': u'active', + u'794dc113-315f-43e3-ae47-0b7b1bf56427': u'active', + u'1d8235ee-c2ce-4c06-abd5-63b655fd66c5': u'active'}, + u'storagepoolID': u'636d9c59-f7ba-4115-87a1-44d6563a9610', + u'scsiKey': u'636d9c59-f7ba-4115-87a1-44d6563a9610', + u'masterSdUUID': u'3d4accf7-74af-4ea0-b59b-eb9f10eedb83', + u'hostID': 1} + + _schema.schema().verify_args( + vdsmapi.MethodRep('StoragePool', 'connect'), params)
gerrit-hooks has posted comments on this change.
Change subject: yml: parameter type fixes for StoragePool.connect ......................................................................
Patch Set 1:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Piotr Kliczewski has posted comments on this change.
Change subject: yml: parameter type fixes for StoragePool.connect ......................................................................
Patch Set 1: Verified+1
Verified by running unit test. The message was copied from Nir's vdsm log.
Freddy Rolland has posted comments on this change.
Change subject: yml: parameter type fixes for StoragePool.connect ......................................................................
Patch Set 1: Code-Review+1
Nir Soffer has posted comments on this change.
Change subject: yml: parameter type fixes for StoragePool.connect ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
The schema fix looks good, I don't understand why we need the tests.
https://gerrit.ovirt.org/#/c/59702/1/tests/vdsmapi_test.py File tests/vdsmapi_test.py:
Line 514: u'masterSdUUID': u'3d4accf7-74af-4ea0-b59b-eb9f10eedb83', Line 515: u'hostID': 1} Line 516: Line 517: _schema.schema().verify_args( Line 518: vdsmapi.MethodRep('StoragePool', 'connect'), params) What does this test? are we expecting this to raise if the schema does match params?
What if we change the schema - do we have to change params? I don't see the value in these tests, they are even harmful.
We should have tests for verify_args, but we don't need to test our schema.
Piotr Kliczewski has posted comments on this change.
Change subject: yml: parameter type fixes for StoragePool.connect ......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/59702/1/tests/vdsmapi_test.py File tests/vdsmapi_test.py:
Line 514: u'masterSdUUID': u'3d4accf7-74af-4ea0-b59b-eb9f10eedb83', Line 515: u'hostID': 1} Line 516: Line 517: _schema.schema().verify_args( Line 518: vdsmapi.MethodRep('StoragePool', 'connect'), params)
What does this test? are we expecting this to raise if the schema does matc
Whenever there is inconsistency between data and schema we raise JsonRpcInvalidParamsError. This test runs verify_args which was a request from the engine.
Piotr Kliczewski has posted comments on this change.
Change subject: yml: parameter type fixes for StoragePool.connect ......................................................................
Patch Set 2: Verified+1
Verified by running schema verification tests
Piotr Kliczewski has posted comments on this change.
Change subject: yml: parameter type fixes for StoragePool.connect ......................................................................
Patch Set 2:
Let's keep this patch for some time longer
Yaniv Bronhaim has posted comments on this change.
Change subject: yml: parameter type fixes for StoragePool.connect ......................................................................
Patch Set 2: Code-Review+1
Adam Litke has posted comments on this change.
Change subject: yml: parameter type fixes for StoragePool.connect ......................................................................
Patch Set 2: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/59702/2/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
PS2, Line 5616: active: The domain is attached to a Storage Pool and is activated : attached: The domain is attached to a Storage Pool but is : deactivated : unattached: The domain is not attached to a Storage Pool : unknown: The status of the Storage Domain is not known Did you verify that these are passed and returned as lowercase values? From my reading of the code these values are capitalized :( Very unfortunate.
From Yaniv Bronhaim ybronhei@redhat.com:
Yaniv Bronhaim has posted comments on this change.
Change subject: yml: parameter type fixes for StoragePool.connect ......................................................................
Patch Set 3: Code-Review+1
vdsm-patches@lists.fedorahosted.org