From fcf6a8eb045afd7bb88dd7ecb80799247a42ce10 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 31 Oct 2016 14:46:43 -0700 Subject: [PATCH 1/4] Updating list_metrics() to Iterator pattern. --- logging/google/cloud/logging/_gax.py | 32 +++++++++++++----- logging/google/cloud/logging/client.py | 13 +++----- logging/google/cloud/logging/connection.py | 39 ++++++++++++++-------- 3 files changed, 54 insertions(+), 30 deletions(-) diff --git a/logging/google/cloud/logging/_gax.py b/logging/google/cloud/logging/_gax.py index 1825e48cb94e..72685c3fca94 100644 --- a/logging/google/cloud/logging/_gax.py +++ b/logging/google/cloud/logging/_gax.py @@ -33,6 +33,7 @@ from google.cloud.iterator import GAXIterator from google.cloud.logging._helpers import entry_from_resource from google.cloud.logging.sink import Sink +from google.cloud.logging.metric import Metric class _LoggingAPI(object): @@ -316,10 +317,10 @@ def list_metrics(self, project, page_size=0, page_token=None): passed, the API will return the first page of metrics. - :rtype: tuple, (list, str) - :returns: list of mappings, plus a "next page token" string: - if not None, indicates that more metrics can be retrieved - with another call (pass that value as ``page_token``). + :rtype: :class:`~google.cloud.iterator.Iterator` + :returns: Iterator of + :class:`~google.cloud.logging.metric.Metric` + accessible to the current API. """ if page_token is None: page_token = INITIAL_PAGE @@ -327,10 +328,7 @@ def list_metrics(self, project, page_size=0, page_token=None): path = 'projects/%s' % (project,) page_iter = self._gax_api.list_log_metrics( path, page_size=page_size, options=options) - metrics = [MessageToDict(log_metric_pb) - for log_metric_pb in page_iter.next()] - token = page_iter.page_token or None - return metrics, token + return GAXIterator(self._client, page_iter, _item_to_metric) def metric_create(self, project, metric_name, filter_, description): """API call: create a metric resource. @@ -496,3 +494,21 @@ def _item_to_sink(iterator, log_sink_pb): """ resource = MessageToDict(log_sink_pb) return Sink.from_api_repr(resource, iterator.client) + + + +def _item_to_metric(iterator, log_metric_pb): + """Convert a metric protobuf to the native object. + + :type iterator: :class:`~google.cloud.iterator.Iterator` + :param iterator: The iterator that is currently in use. + + :type log_metric_pb: + :class:`~google.logging.v2.logging_metrics_pb2.LogMetric` + :param log_metric_pb: Metric protobuf returned from the API. + + :rtype: :class:`~google.cloud.logging.metric.Metric` + :returns: The next metric in the page. + """ + resource = MessageToDict(log_metric_pb) + return Metric.from_api_repr(resource, iterator.client) diff --git a/logging/google/cloud/logging/client.py b/logging/google/cloud/logging/client.py index 692b3c6a0c12..ec3cf8174719 100644 --- a/logging/google/cloud/logging/client.py +++ b/logging/google/cloud/logging/client.py @@ -267,14 +267,9 @@ def list_metrics(self, page_size=None, page_token=None): passed, the API will return the first page of metrics. - :rtype: tuple, (list, str) - :returns: list of :class:`google.cloud.logging.metric.Metric`, plus a - "next page token" string: if not None, indicates that - more metrics can be retrieved with another call (pass that - value as ``page_token``). + :rtype: :class:`~google.cloud.iterator.Iterator` + :returns: Iterator of :class:`~google.cloud.logging.metric.Metric` + accessible to the current client. """ - resources, token = self.metrics_api.list_metrics( + return self.metrics_api.list_metrics( self.project, page_size, page_token) - metrics = [Metric.from_api_repr(resource, self) - for resource in resources] - return metrics, token diff --git a/logging/google/cloud/logging/connection.py b/logging/google/cloud/logging/connection.py index 2f50eb988cbc..a1fa388b0f09 100644 --- a/logging/google/cloud/logging/connection.py +++ b/logging/google/cloud/logging/connection.py @@ -20,6 +20,7 @@ from google.cloud.iterator import HTTPIterator from google.cloud.logging._helpers import entry_from_resource from google.cloud.logging.sink import Sink +from google.cloud.logging.metric import Metric class Connection(base_connection.JSONConnection): @@ -347,24 +348,21 @@ def list_metrics(self, project, page_size=None, page_token=None): passed, the API will return the first page of metrics. - :rtype: tuple, (list, str) - :returns: list of mappings, plus a "next page token" string: - if not None, indicates that more metrics can be retrieved - with another call (pass that value as ``page_token``). + :rtype: :class:`~google.cloud.iterator.Iterator` + :returns: Iterator of + :class:`~google.cloud.logging.metric.Metric` + accessible to the current API. """ - params = {} + extra_params = {} if page_size is not None: - params['pageSize'] = page_size - - if page_token is not None: - params['pageToken'] = page_token + extra_params['pageSize'] = page_size path = '/projects/%s/metrics' % (project,) - resp = self._connection.api_request( - method='GET', path=path, query_params=params) - metrics = resp.get('metrics', ()) - return metrics, resp.get('nextPageToken') + return HTTPIterator( + client=self._client, path=path, + item_to_value=_item_to_metric, items_key='metrics', + page_token=page_token, extra_params=extra_params) def metric_create(self, project, metric_name, filter_, description=None): """API call: create a metric resource. @@ -497,3 +495,18 @@ def _item_to_sink(iterator, resource): :returns: The next sink in the page. """ return Sink.from_api_repr(resource, iterator.client) + + +def _item_to_metric(iterator, resource): + """Convert a metric resource to the native object. + + :type iterator: :class:`~google.cloud.iterator.Iterator` + :param iterator: The iterator that is currently in use. + + :type resource: dict + :param resource: Metric JSON resource returned from the API. + + :rtype: :class:`~google.cloud.logging.metric.Metric` + :returns: The next metric in the page. + """ + return Metric.from_api_repr(resource, iterator.client) From bd179bc63cb4afa8e6c337d340c38c27f6fca855 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 31 Oct 2016 15:48:37 -0700 Subject: [PATCH 2/4] Updating system test and docs for list_metrics() change. --- docs/logging-usage.rst | 2 +- system_tests/logging_.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/logging-usage.rst b/docs/logging-usage.rst index 062d677c1fc3..e7aa16630c09 100644 --- a/docs/logging-usage.rst +++ b/docs/logging-usage.rst @@ -171,7 +171,7 @@ List all metrics for a project: >>> from google.cloud import logging >>> client = logging.Client() - >>> metrics, token = client.list_metrics() + >>> metrics = list(client.list_metrics()) >>> len(metrics) 1 >>> metric = metrics[0] diff --git a/system_tests/logging_.py b/system_tests/logging_.py index f3231abeea42..3f23c204d24d 100644 --- a/system_tests/logging_.py +++ b/system_tests/logging_.py @@ -267,12 +267,12 @@ def test_list_metrics(self): metric = Config.CLIENT.metric( METRIC_NAME, DEFAULT_FILTER, DEFAULT_DESCRIPTION) self.assertFalse(metric.exists()) - before_metrics, _ = Config.CLIENT.list_metrics() + before_metrics = list(Config.CLIENT.list_metrics()) before_names = set(metric.name for metric in before_metrics) metric.create() self.to_delete.append(metric) self.assertTrue(metric.exists()) - after_metrics, _ = Config.CLIENT.list_metrics() + after_metrics = list(Config.CLIENT.list_metrics()) after_names = set(metric.name for metric in after_metrics) self.assertEqual(after_names - before_names, set([METRIC_NAME])) @@ -304,7 +304,7 @@ def test_update_metric(self): metric.filter_ = NEW_FILTER metric.description = NEW_DESCRIPTION metric.update() - after_metrics, _ = Config.CLIENT.list_metrics() + after_metrics = list(Config.CLIENT.list_metrics()) after_info = {metric.name: metric for metric in after_metrics} after = after_info[METRIC_NAME] self.assertEqual(after.filter_, NEW_FILTER) From 9619426e18103d2eea5073a95a8cf0790bcc4440 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 31 Oct 2016 16:15:13 -0700 Subject: [PATCH 3/4] Updating unit tests are list_metrics() iterator update. --- logging/google/cloud/logging/_gax.py | 1 - logging/unit_tests/test__gax.py | 52 ++++++++++++------- logging/unit_tests/test_client.py | 75 +++++++++++++++++++-------- logging/unit_tests/test_connection.py | 55 ++++++++++++++++---- 4 files changed, 132 insertions(+), 51 deletions(-) diff --git a/logging/google/cloud/logging/_gax.py b/logging/google/cloud/logging/_gax.py index 72685c3fca94..5e096841324c 100644 --- a/logging/google/cloud/logging/_gax.py +++ b/logging/google/cloud/logging/_gax.py @@ -496,7 +496,6 @@ def _item_to_sink(iterator, log_sink_pb): return Sink.from_api_repr(resource, iterator.client) - def _item_to_metric(iterator, log_metric_pb): """Convert a metric protobuf to the native object. diff --git a/logging/unit_tests/test__gax.py b/logging/unit_tests/test__gax.py index ed5f1f12c9f0..90206e26a388 100644 --- a/logging/unit_tests/test__gax.py +++ b/logging/unit_tests/test__gax.py @@ -843,27 +843,36 @@ def test_ctor(self): self.assertIs(api._gax_api, gax_api) def test_list_metrics_no_paging(self): + import six from google.gax import INITIAL_PAGE - from google.cloud._testing import _GAXPageIterator from google.logging.v2.logging_metrics_pb2 import LogMetric + from google.cloud._testing import _GAXPageIterator + from google.cloud.logging.metric import Metric TOKEN = 'TOKEN' - METRICS = [{ - 'name': self.METRIC_PATH, - 'filter': self.FILTER, - 'description': self.DESCRIPTION, - }] metric_pb = LogMetric(name=self.METRIC_PATH, description=self.DESCRIPTION, filter=self.FILTER) response = _GAXPageIterator([metric_pb], page_token=TOKEN) gax_api = _GAXMetricsAPI(_list_log_metrics_response=response) - api = self._makeOne(gax_api, None) + client = object() + api = self._makeOne(gax_api, client) - metrics, token = api.list_metrics(self.PROJECT) + iterator = api.list_metrics(self.PROJECT) + page = six.next(iterator.pages) + metrics = list(page) + token = iterator.next_page_token - self.assertEqual(metrics, METRICS) + # First check the token. self.assertEqual(token, TOKEN) + # Then check the metrics returned. + self.assertEqual(len(metrics), 1) + metric = metrics[0] + self.assertIsInstance(metric, Metric) + self.assertEqual(metric.name, self.METRIC_PATH) + self.assertEqual(metric.filter_, self.FILTER) + self.assertEqual(metric.description, self.DESCRIPTION) + self.assertIs(metric.client, client) project, page_size, options = gax_api._list_log_metrics_called_with self.assertEqual(project, self.PROJECT_PATH) @@ -871,28 +880,35 @@ def test_list_metrics_no_paging(self): self.assertEqual(options.page_token, INITIAL_PAGE) def test_list_metrics_w_paging(self): - from google.cloud._testing import _GAXPageIterator from google.logging.v2.logging_metrics_pb2 import LogMetric + from google.cloud._testing import _GAXPageIterator + from google.cloud.logging.metric import Metric TOKEN = 'TOKEN' PAGE_SIZE = 42 - METRICS = [{ - 'name': self.METRIC_PATH, - 'filter': self.FILTER, - 'description': self.DESCRIPTION, - }] metric_pb = LogMetric(name=self.METRIC_PATH, description=self.DESCRIPTION, filter=self.FILTER) response = _GAXPageIterator([metric_pb]) gax_api = _GAXMetricsAPI(_list_log_metrics_response=response) - api = self._makeOne(gax_api, None) + client = object() + api = self._makeOne(gax_api, client) - metrics, token = api.list_metrics( + iterator = api.list_metrics( self.PROJECT, page_size=PAGE_SIZE, page_token=TOKEN) + metrics = list(iterator) + token = iterator.next_page_token - self.assertEqual(metrics, METRICS) + # First check the token. self.assertIsNone(token) + # Then check the metrics returned. + self.assertEqual(len(metrics), 1) + metric = metrics[0] + self.assertIsInstance(metric, Metric) + self.assertEqual(metric.name, self.METRIC_PATH) + self.assertEqual(metric.filter_, self.FILTER) + self.assertEqual(metric.description, self.DESCRIPTION) + self.assertIs(metric.client, client) project, page_size, options = gax_api._list_log_metrics_called_with self.assertEqual(project, self.PROJECT_PATH) diff --git a/logging/unit_tests/test_client.py b/logging/unit_tests/test_client.py index 8c94a088bbcc..490372f03a96 100644 --- a/logging/unit_tests/test_client.py +++ b/logging/unit_tests/test_client.py @@ -485,7 +485,9 @@ def test_metric_explicit(self): self.assertEqual(metric.project, self.PROJECT) def test_list_metrics_no_paging(self): + import six from google.cloud.logging.metric import Metric + PROJECT = 'PROJECT' TOKEN = 'TOKEN' METRICS = [{ @@ -493,21 +495,39 @@ def test_list_metrics_no_paging(self): 'filter': self.FILTER, 'description': self.DESCRIPTION, }] - client = self._makeOne(project=PROJECT, credentials=_Credentials()) - api = client._metrics_api = _DummyMetricsAPI() - api._list_metrics_response = METRICS, TOKEN + client = self._makeOne(project=PROJECT, credentials=_Credentials(), + use_gax=False) + returned = { + 'metrics': METRICS, + 'nextPageToken': TOKEN, + } + client.connection = _Connection(returned) - metrics, token = client.list_metrics() + # Execute request. + iterator = client.list_metrics() + page = six.next(iterator.pages) + metrics = list(page) + token = iterator.next_page_token + # First check the token. + self.assertEqual(token, TOKEN) + # Then check the metrics returned. self.assertEqual(len(metrics), 1) metric = metrics[0] self.assertIsInstance(metric, Metric) self.assertEqual(metric.name, self.METRIC_NAME) self.assertEqual(metric.filter_, self.FILTER) self.assertEqual(metric.description, self.DESCRIPTION) - self.assertEqual(token, TOKEN) - self.assertEqual(api._list_metrics_called_with, - (PROJECT, None, None)) + self.assertIs(metric.client, client) + + # Verify mocked transport. + called_with = client.connection._called_with + path = '/projects/%s/metrics' % (self.PROJECT,) + self.assertEqual(called_with, { + 'method': 'GET', + 'path': path, + 'query_params': {}, + }) def test_list_metrics_with_paging(self): from google.cloud.logging.metric import Metric @@ -519,22 +539,40 @@ def test_list_metrics_with_paging(self): 'filter': self.FILTER, 'description': self.DESCRIPTION, }] - client = self._makeOne(project=PROJECT, credentials=_Credentials()) - api = client._metrics_api = _DummyMetricsAPI() - api._list_metrics_response = METRICS, None + client = self._makeOne(project=PROJECT, credentials=_Credentials(), + use_gax=False) + returned = { + 'metrics': METRICS, + } + client.connection = _Connection(returned) # Execute request. - metrics, token = client.list_metrics(PAGE_SIZE, TOKEN) - # Test values are correct. + iterator = client.list_metrics(PAGE_SIZE, TOKEN) + metrics = list(iterator) + token = iterator.next_page_token + + # First check the token. + self.assertIsNone(token) + # Then check the metrics returned. self.assertEqual(len(metrics), 1) metric = metrics[0] self.assertIsInstance(metric, Metric) self.assertEqual(metric.name, self.METRIC_NAME) self.assertEqual(metric.filter_, self.FILTER) self.assertEqual(metric.description, self.DESCRIPTION) - self.assertIsNone(token) - self.assertEqual(api._list_metrics_called_with, - (PROJECT, PAGE_SIZE, TOKEN)) + self.assertIs(metric.client, client) + + # Verify mocked transport. + called_with = client.connection._called_with + path = '/projects/%s/metrics' % (self.PROJECT,) + self.assertEqual(called_with, { + 'method': 'GET', + 'path': path, + 'query_params': { + 'pageSize': PAGE_SIZE, + 'pageToken': TOKEN, + }, + }) class _Credentials(object): @@ -550,13 +588,6 @@ def create_scoped(self, scope): return self -class _DummyMetricsAPI(object): - - def list_metrics(self, project, page_size, page_token): - self._list_metrics_called_with = (project, page_size, page_token) - return self._list_metrics_response - - class _Connection(object): _called_with = None diff --git a/logging/unit_tests/test_connection.py b/logging/unit_tests/test_connection.py index ec25c185e759..5d41877476d8 100644 --- a/logging/unit_tests/test_connection.py +++ b/logging/unit_tests/test_connection.py @@ -540,6 +540,9 @@ def _makeOne(self, *args, **kw): return self._getTargetClass()(*args, **kw) def test_list_metrics_no_paging(self): + import six + from google.cloud.logging.metric import Metric + TOKEN = 'TOKEN' RETURNED = { 'metrics': [{ @@ -552,16 +555,33 @@ def test_list_metrics_no_paging(self): client = _Client(conn) api = self._makeOne(client) - metrics, token = api.list_metrics(self.PROJECT) + iterator = api.list_metrics(self.PROJECT) + page = six.next(iterator.pages) + metrics = list(page) + token = iterator.next_page_token - self.assertEqual(metrics, RETURNED['metrics']) + # First check the token. self.assertEqual(token, TOKEN) + # Then check the metrics returned. + self.assertEqual(len(metrics), 1) + metric = metrics[0] + self.assertIsInstance(metric, Metric) + self.assertEqual(metric.name, self.METRIC_PATH) + self.assertEqual(metric.filter_, self.FILTER) + self.assertEqual(metric.description, '') + self.assertIs(metric.client, client) - self.assertEqual(conn._called_with['method'], 'GET') + called_with = conn._called_with path = '/%s' % (self.LIST_METRICS_PATH,) - self.assertEqual(conn._called_with['path'], path) + self.assertEqual(called_with, { + 'method': 'GET', + 'path': path, + 'query_params': {}, + }) def test_list_metrics_w_paging(self): + from google.cloud.logging.metric import Metric + TOKEN = 'TOKEN' PAGE_SIZE = 42 RETURNED = { @@ -574,17 +594,32 @@ def test_list_metrics_w_paging(self): client = _Client(conn) api = self._makeOne(client) - metrics, token = api.list_metrics( + iterator = api.list_metrics( self.PROJECT, page_size=PAGE_SIZE, page_token=TOKEN) + metrics = list(iterator) + token = iterator.next_page_token - self.assertEqual(metrics, RETURNED['metrics']) + # First check the token. self.assertIsNone(token) + # Then check the metrics returned. + self.assertEqual(len(metrics), 1) + metric = metrics[0] + self.assertIsInstance(metric, Metric) + self.assertEqual(metric.name, self.METRIC_PATH) + self.assertEqual(metric.filter_, self.FILTER) + self.assertEqual(metric.description, '') + self.assertIs(metric.client, client) - self.assertEqual(conn._called_with['method'], 'GET') + called_with = conn._called_with path = '/%s' % (self.LIST_METRICS_PATH,) - self.assertEqual(conn._called_with['path'], path) - self.assertEqual(conn._called_with['query_params'], - {'pageSize': PAGE_SIZE, 'pageToken': TOKEN}) + self.assertEqual(called_with, { + 'method': 'GET', + 'path': path, + 'query_params': { + 'pageSize': PAGE_SIZE, + 'pageToken': TOKEN, + }, + }) def test_metric_create_conflict(self): from google.cloud.exceptions import Conflict From 9cf835c08cbd4b5affdd9c4ebd7f354245ae6c79 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Tue, 1 Nov 2016 09:23:49 -0700 Subject: [PATCH 4/4] Updating list_metrics() unit tests to reflect paging / non-paging. --- logging/unit_tests/test_client.py | 51 +++++++++++++++---------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/logging/unit_tests/test_client.py b/logging/unit_tests/test_client.py index 490372f03a96..609d8b40728e 100644 --- a/logging/unit_tests/test_client.py +++ b/logging/unit_tests/test_client.py @@ -485,33 +485,26 @@ def test_metric_explicit(self): self.assertEqual(metric.project, self.PROJECT) def test_list_metrics_no_paging(self): - import six from google.cloud.logging.metric import Metric - PROJECT = 'PROJECT' - TOKEN = 'TOKEN' - METRICS = [{ + metrics = [{ 'name': self.METRIC_NAME, 'filter': self.FILTER, 'description': self.DESCRIPTION, }] - client = self._makeOne(project=PROJECT, credentials=_Credentials(), - use_gax=False) + client = self._makeOne( + project=self.PROJECT, credentials=_Credentials(), + use_gax=False) returned = { - 'metrics': METRICS, - 'nextPageToken': TOKEN, + 'metrics': metrics, } client.connection = _Connection(returned) # Execute request. iterator = client.list_metrics() - page = six.next(iterator.pages) - metrics = list(page) - token = iterator.next_page_token + metrics = list(iterator) - # First check the token. - self.assertEqual(token, TOKEN) - # Then check the metrics returned. + # Check the metrics returned. self.assertEqual(len(metrics), 1) metric = metrics[0] self.assertIsInstance(metric, Metric) @@ -530,29 +523,33 @@ def test_list_metrics_no_paging(self): }) def test_list_metrics_with_paging(self): + import six from google.cloud.logging.metric import Metric - PROJECT = 'PROJECT' - TOKEN = 'TOKEN' - PAGE_SIZE = 42 - METRICS = [{ + + token = 'TOKEN' + next_token = 'T00KEN' + page_size = 42 + metrics = [{ 'name': self.METRIC_NAME, 'filter': self.FILTER, 'description': self.DESCRIPTION, }] - client = self._makeOne(project=PROJECT, credentials=_Credentials(), - use_gax=False) + client = self._makeOne( + project=self.PROJECT, credentials=_Credentials(), + use_gax=False) returned = { - 'metrics': METRICS, + 'metrics': metrics, + 'nextPageToken': next_token, } client.connection = _Connection(returned) # Execute request. - iterator = client.list_metrics(PAGE_SIZE, TOKEN) - metrics = list(iterator) - token = iterator.next_page_token + iterator = client.list_metrics(page_size, token) + page = six.next(iterator.pages) + metrics = list(page) # First check the token. - self.assertIsNone(token) + self.assertEqual(iterator.next_page_token, next_token) # Then check the metrics returned. self.assertEqual(len(metrics), 1) metric = metrics[0] @@ -569,8 +566,8 @@ def test_list_metrics_with_paging(self): 'method': 'GET', 'path': path, 'query_params': { - 'pageSize': PAGE_SIZE, - 'pageToken': TOKEN, + 'pageSize': page_size, + 'pageToken': token, }, })