x/python-cratonclient
Code Issues Proposed changes

Enable less code in cratonclient resources

Browse Source 
Previously, resource managers were overriding __init__, list, and create
to include extra parameters, e.g., region_id. This slightly alters the
base CRUDClient to accept extra request keyword arguments to merge into
each request's arguments.

Change-Id: Ia850417c7a94eea95d35c8a93141c70945d1a800
This commit is contained in:
Ian Cordasco
2016-10-26 12:30:24 -05:00
parent a42802fb04
commit 68df0e30c7
8 changed files with 84 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
cratonclient/crud.py
View File

@@ -24,10 +24,11 @@ class CRUDClient(object):
base_path = None base_path = None
resource_class = None resource_class = None
def __init__(self, session, url): def __init__(self, session, url, **extra_request_kwargs):
"""Initialize our Client with a session and base url.""" """Initialize our Client with a session and base url."""
self.session = session self.session = session
self.url = url.rstrip('/') self.url = url.rstrip('/')
self.extra_request_kwargs = extra_request_kwargs
def build_url(self, path_arguments=None): def build_url(self, path_arguments=None):
"""Build a complete URL from the url, base_path, and arguments. """Build a complete URL from the url, base_path, and arguments.
@@ -76,21 +77,34 @@ class CRUDClient(object):
return url return url
def create(self, **kwargs): def merge_request_arguments(self, request_kwargs, skip_merge):
"""Merge the extra request arguments into the per-request args."""
if skip_merge:
return
keys = set(self.extra_request_kwargs.keys())
missing_keys = keys.difference(request_kwargs.keys())
for key in missing_keys:
request_kwargs[key] = self.extra_request_kwargs[key]
def create(self, skip_merge=False, **kwargs):
"""Create a new item based on the keyword arguments provided.""" """Create a new item based on the keyword arguments provided."""
self.merge_request_arguments(kwargs, skip_merge)
url = self.build_url(path_arguments=kwargs) url = self.build_url(path_arguments=kwargs)
response = self.session.post(url, json=kwargs) response = self.session.post(url, json=kwargs)
return self.resource_class(self, response.json(), loaded=True) return self.resource_class(self, response.json(), loaded=True)
def get(self, item_id=None, **kwargs): def get(self, item_id=None, skip_merge=True, **kwargs):
"""Retrieve the item based on the keyword arguments provided.""" """Retrieve the item based on the keyword arguments provided."""
self.merge_request_arguments(kwargs, skip_merge)
kwargs.setdefault(self.key + '_id', item_id) kwargs.setdefault(self.key + '_id', item_id)
url = self.build_url(path_arguments=kwargs) url = self.build_url(path_arguments=kwargs)
response = self.session.get(url) response = self.session.get(url)
return self.resource_class(self, response.json(), loaded=True) return self.resource_class(self, response.json(), loaded=True)
def list(self, **kwargs): def list(self, skip_merge=False, **kwargs):
"""List the items from this endpoint.""" """List the items from this endpoint."""
self.merge_request_arguments(kwargs, skip_merge)
url = self.build_url(path_arguments=kwargs) url = self.build_url(path_arguments=kwargs)
response = self.session.get(url, params=kwargs) response = self.session.get(url, params=kwargs)
return [ return [
@@ -98,15 +112,17 @@ class CRUDClient(object):
for item in response.json() for item in response.json()
] ]
def update(self, item_id=None, **kwargs): def update(self, item_id=None, skip_merge=True, **kwargs):
"""Update the item based on the keyword arguments provided.""" """Update the item based on the keyword arguments provided."""
self.merge_request_arguments(kwargs, skip_merge)
kwargs.setdefault(self.key + '_id', item_id) kwargs.setdefault(self.key + '_id', item_id)
url = self.build_url(path_arguments=kwargs) url = self.build_url(path_arguments=kwargs)
response = self.session.put(url, json=kwargs) response = self.session.put(url, json=kwargs)
return self.resource_class(self, response.json(), loaded=True) return self.resource_class(self, response.json(), loaded=True)
def delete(self, item_id=None, **kwargs): def delete(self, item_id=None, skip_merge=True, **kwargs):
"""Delete the item based on the keyword arguments provided.""" """Delete the item based on the keyword arguments provided."""
self.merge_request_arguments(kwargs, skip_merge)
kwargs.setdefault(self.key + '_id', item_id) kwargs.setdefault(self.key + '_id', item_id)
url = self.build_url(path_arguments=kwargs) url = self.build_url(path_arguments=kwargs)
response = self.session.delete(url, params=kwargs) response = self.session.delete(url, params=kwargs)

24
cratonclient/tests/unit/test_cells_shell.py
View File

@@ -157,8 +157,8 @@ class TestCellsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.cells = cells.CellManager(mock.ANY, inventory.cells = cells.CellManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
cells_shell.do_cell_create(client, self.cell_valid_fields) cells_shell.do_cell_create(client, self.cell_valid_fields)
@@ -170,8 +170,8 @@ class TestCellsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.cells = cells.CellManager(mock.ANY, inventory.cells = cells.CellManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
cells_shell.do_cell_create(client, self.cell_invalid_field) cells_shell.do_cell_create(client, self.cell_invalid_field)
@@ -195,8 +195,8 @@ class TestCellsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.cells = cells.CellManager(mock.ANY, inventory.cells = cells.CellManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
valid_input = Namespace(region=1, valid_input = Namespace(region=1,
@@ -212,8 +212,8 @@ class TestCellsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.cells = cells.CellManager(mock.ANY, inventory.cells = cells.CellManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
invalid_input = Namespace(region=1, invalid_input = Namespace(region=1,
@@ -243,8 +243,8 @@ class TestCellsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.cells = cells.CellManager(mock.ANY, inventory.cells = cells.CellManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
test_args = Namespace(id=1, region=1) test_args = Namespace(id=1, region=1)
@@ -268,8 +268,8 @@ class TestCellsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.cells = cells.CellManager(mock.ANY, inventory.cells = cells.CellManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
test_args = Namespace(id=1, region=1) test_args = Namespace(id=1, region=1)

30
cratonclient/tests/unit/test_crud.py
View File

@@ -30,10 +30,16 @@ class TestCRUDClient(base.TestCase):
super(TestCRUDClient, self).setUp() super(TestCRUDClient, self).setUp()
self.session = mock.Mock() self.session = mock.Mock()
self.resource_spec = mock.Mock(spec=crud.Resource) self.resource_spec = mock.Mock(spec=crud.Resource)
self.client = crud.CRUDClient(self.session, 'http://example.com/v1/') self.client = self.create_client()
self.client.base_path = '/test'
self.client.key = 'test_key' def create_client(self, **kwargs):
self.client.resource_class = self.resource_spec """Create and configure a basic CRUDClient."""
client = crud.CRUDClient(self.session, 'http://example.com/v1/',
**kwargs)
client.base_path = '/test'
client.key = 'test_key'
client.resource_class = self.resource_spec
return client
def test_strips_trailing_forward_slash_from_url(self): def test_strips_trailing_forward_slash_from_url(self):
"""Verify the client strips the trailing / in a URL.""" """Verify the client strips the trailing / in a URL."""
@@ -73,6 +79,22 @@ class TestCRUDClient(base.TestCase):
}), }),
) )
def test_merge_request_arguments(self):
"""Verify we include extra request arguments."""
client = self.create_client(extra_id=4321)
request_args = {}
client.merge_request_arguments(request_args, skip_merge=False)
self.assertEqual({'extra_id': 4321}, request_args)
def test_merge_request_arguments_skips_merging(self):
"""Verify we include extra request arguments."""
client = self.create_client(extra_id=4321)
request_args = {}
client.merge_request_arguments(request_args, skip_merge=True)
self.assertEqual({}, request_args)
def test_create_generates_a_post_request(self): def test_create_generates_a_post_request(self):
"""Verify that using our create method will POST to our service.""" """Verify that using our create method will POST to our service."""
response = self.session.post.return_value = mock.Mock() response = self.session.post.return_value = mock.Mock()

24
cratonclient/tests/unit/test_hosts_shell.py
View File

@@ -169,8 +169,8 @@ class TestHostsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.hosts = hosts.HostManager(mock.ANY, inventory.hosts = hosts.HostManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
hosts_shell.do_host_create(client, self.host_valid_fields) hosts_shell.do_host_create(client, self.host_valid_fields)
@@ -182,8 +182,8 @@ class TestHostsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.hosts = hosts.HostManager(mock.ANY, inventory.hosts = hosts.HostManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
hosts_shell.do_host_create(client, self.host_invalid_field) hosts_shell.do_host_create(client, self.host_invalid_field)
@@ -207,8 +207,8 @@ class TestHostsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.hosts = hosts.HostManager(mock.ANY, inventory.hosts = hosts.HostManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
valid_input = Namespace(region=1, valid_input = Namespace(region=1,
@@ -224,8 +224,8 @@ class TestHostsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.hosts = hosts.HostManager(mock.ANY, inventory.hosts = hosts.HostManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
invalid_input = Namespace(region=1, invalid_input = Namespace(region=1,
@@ -255,8 +255,8 @@ class TestHostsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.hosts = hosts.HostManager(mock.ANY, inventory.hosts = hosts.HostManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
test_args = Namespace(id=1, region=1) test_args = Namespace(id=1, region=1)
@@ -280,8 +280,8 @@ class TestHostsShell(base.ShellTestCase):
client = mock.Mock() client = mock.Mock()
inventory = mock.Mock() inventory = mock.Mock()
inventory.hosts = hosts.HostManager(mock.ANY, inventory.hosts = hosts.HostManager(mock.ANY,
mock.ANY, 'http://127.0.0.1/',
'http://127.0.0.1/') region_id=mock.ANY)
client.inventory = mock.Mock(name='inventory') client.inventory = mock.Mock(name='inventory')
client.inventory.return_value = inventory client.inventory.return_value = inventory
test_args = Namespace(id=1, region=1) test_args = Namespace(id=1, region=1)

12
cratonclient/tests/unit/test_inventory.py
View File

@@ -28,7 +28,11 @@ class TestInventory(base.TestCase):
url = 'https://10.1.1.0:8080/' url = 'https://10.1.1.0:8080/'
region_id = 1 region_id = 1
inventory.Inventory(session, url, region_id) inventory.Inventory(session, url, region_id)
mock_hostmanager.assert_called_once_with(region_id, session, url) mock_hostmanager.assert_called_once_with(
session,
url,
region_id=region_id,
)
@mock.patch('cratonclient.v1.cells.CellManager') @mock.patch('cratonclient.v1.cells.CellManager')
def test_inventory_creates_cell_manager(self, cell_manager): def test_inventory_creates_cell_manager(self, cell_manager):
@@ -37,4 +41,8 @@ class TestInventory(base.TestCase):
url = 'https://10.1.1.0:8080/' url = 'https://10.1.1.0:8080/'
region_id = 1 region_id = 1
inventory.Inventory(session, url, region_id) inventory.Inventory(session, url, region_id)
cell_manager.assert_called_once_with(region_id, session, url) cell_manager.assert_called_once_with(
session,
url,
region_id=region_id,
)

16
cratonclient/v1/cells.py
View File

@@ -27,22 +27,6 @@ class CellManager(crud.CRUDClient):
key = 'cell' key = 'cell'
base_path = '/cells' base_path = '/cells'
resource_class = Cell resource_class = Cell
region_id = 0
def __init__(self, region_id, session, url):
"""Initialize our CellManager object with region, session, and url."""
super(CellManager, self).__init__(session, url)
self.region_id = region_id
def list(self, **kwargs):
"""Retrieve the cells in a specific region."""
kwargs['region_id'] = self.region_id
return super(CellManager, self).list(**kwargs)
def create(self, **kwargs):
"""Create a cell in a specific region."""
kwargs['region_id'] = self.region_id
return super(CellManager, self).create(**kwargs)
CELL_FIELDS = { CELL_FIELDS = {

16
cratonclient/v1/hosts.py
View File

@@ -27,22 +27,6 @@ class HostManager(crud.CRUDClient):
key = 'host' key = 'host'
base_path = '/hosts' base_path = '/hosts'
resource_class = Host resource_class = Host
region_id = 0
def __init__(self, region_id, session, url):
"""Initialize our HostManager object with region, session and url."""
super(HostManager, self).__init__(session, url)
self.region_id = region_id
def list(self, **kwargs):
"""Retrieve the hosts in a specific region."""
kwargs['region_id'] = self.region_id
return super(HostManager, self).list(**kwargs)
def create(self, **kwargs):
"""Create a host in a specific region."""
kwargs['region_id'] = self.region_id
return super(HostManager, self).create(**kwargs)
HOST_FIELDS = { HOST_FIELDS = {

4
cratonclient/v1/inventory.py
View File

@@ -31,6 +31,6 @@ class Inventory(object):
'https://10.1.1.0:8080/'. 'https://10.1.1.0:8080/'.
""" """
# TODO(cmspence): self.region = self.regions.get(region=region_id) # TODO(cmspence): self.region = self.regions.get(region=region_id)
self.hosts = hosts.HostManager(region_id, session, url) self.hosts = hosts.HostManager(session, url, region_id=region_id)
self.cells = cells.CellManager(region_id, session, url) self.cells = cells.CellManager(session, url, region_id=region_id)
# TODO(cmspence): self.users, self.projects, self.workflows # TODO(cmspence): self.users, self.projects, self.workflows