Throw an exception on conflicting arguments

If session and only_remote conflict, we should throw an error. Otherwise
saying "session=None, only_remote=True" silently does not fetch remotely
and returns builtins.

While we're in there, clean up review feedback.

Change-Id: I70db70a541a3401bb57b2003a90333adb3508b51
This commit is contained in:
Monty Taylor 2017-07-20 06:11:48 +09:00
parent 48a94e00a7
commit 6522198058
No known key found for this signature in database
GPG Key ID: 7BAE94BC7141A594
4 changed files with 20 additions and 27 deletions

View File

@ -39,9 +39,15 @@ class ServiceTypes(object):
remotely the builtin data will be returned as a fallback. only_remote
will cause remote failures to raise an error instead of falling back.
Optional, defaults to False.
:raises ValueError: If session is None and only_remote is True
:raises IOError: If session is given and only_remote is True and there is
an error fetching remote data.
"""
def __init__(self, session=None, only_remote=False):
if not session and only_remote:
raise ValueError(
"only_remote was requested but no Session was provided.")
self._service_types_data = BUILTIN_DATA
if session:
try:
@ -65,10 +71,8 @@ class ServiceTypes(object):
"Convert repo name to project name."
if name is None:
raise ValueError("Empty project name is not allowed")
if name.startswith('openstack/'):
# Handle openstack/ prefix going away from STA data
return name.rpartition('/')[-1]
return name
# Handle openstack/ prefix going away from STA data
return name.rpartition('/')[-1]
@property
def url(self):
@ -193,7 +197,7 @@ class ServiceTypes(object):
return self._service_types_data['reverse'].get(service_type)
def get_all_types(self, service_type):
"""Get a list of official type and all known aliases.
"""Get a list of official types and all known aliases.
:param str service_type: The service-type or alias to get data for.
:returns dict: Service data for the service or None if not found.

View File

@ -20,7 +20,6 @@ import datetime
import keystoneauth1.session
from oslotest import base
from requests_mock.contrib import fixture as rm_fixture
import os_service_types.service_types
@ -34,8 +33,6 @@ class TestCase(base.BaseTestCase):
# use keystoneauth1 to get a Sessiom with no auth information
self.session = keystoneauth1.session.Session()
self.set_adapter()
self.builtin_content = os_service_types.service_types.BUILTIN_DATA
self.builtin_version = self.builtin_content['version']
@ -45,10 +42,6 @@ class TestCase(base.BaseTestCase):
self.remote_content = copy.deepcopy(self.builtin_content)
self.remote_content['version'] = self.remote_version
def set_adapter(self):
# Set up a requests_mock fixture for all HTTP traffic
self.adapter = self.useFixture(rm_fixture.Fixture())
class ServiceDataMixin(object):
@ -122,13 +115,9 @@ class ServiceDataMixin(object):
self.service_types.is_official(self.service_type))
def test_get_project_name(self):
if self.project:
self.assertEqual(
self.project,
self.service_types.get_project_name(self.service_type))
else:
self.assertIsNone(
self.service_types.get_project_name(self.service_type))
self.assertEqual(
self.project,
self.service_types.get_project_name(self.service_type))
def test_get_service_data(self):
service_data = self.service_types.get_service_data(self.service_type)

View File

@ -54,9 +54,6 @@ class TestMatch(base.TestCase):
self.service_types = os_service_types.ServiceTypes()
def test_is_match(self):
if self.is_match:
self.assertTrue(
self.service_types.is_match(self.requested, self.found))
else:
self.assertFalse(
self.service_types.is_match(self.requested, self.found))
self.assertEqual(
self.is_match,
self.service_types.is_match(self.requested, self.found))

View File

@ -21,6 +21,7 @@ Tests for `ServiceTypes` class remote data.
oslotest sets up a TempHomeDir for us, so there should be no ~/.cache files
available in these tests.
"""
from requests_mock.contrib import fixture as rm_fixture
from testscenarios import load_tests_apply_scenarios as load_tests # noqa
import os_service_types
@ -32,14 +33,16 @@ class TestRemote(base.TestCase, base.ServiceDataMixin):
def setUp(self):
super(TestRemote, self).setUp()
self.adapter.register_uri(
# Set up a requests_mock fixture for all HTTP traffic
adapter = self.useFixture(rm_fixture.Fixture())
adapter.register_uri(
'GET', os_service_types.service_types.SERVICE_TYPES_URL,
json=self.remote_content,
headers={'etag': self.getUniqueString('etag')})
# Make an object that fetches from the network
self.service_types = os_service_types.ServiceTypes(
session=self.session)
self.assertEqual(1, len(self.adapter.request_history))
self.assertEqual(1, len(adapter.request_history))
def test_remote_version(self):
self.assertEqual(self.remote_version, self.service_types.version)