From 979f65842436ee31cc2a56fff789010d4e640bee Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 3 Aug 2021 15:14:30 -0700 Subject: [PATCH 01/16] added one consistency test --- tests/system/test_system.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 365e94215..c48f61e3c 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -97,11 +97,13 @@ class Config(object): """ CLIENT = None + HTTP_CLIENT = None use_mtls = os.environ.get("GOOGLE_API_USE_MTLS_ENDPOINT", "never") def setUpModule(): Config.CLIENT = client.Client() + Config.HTTP_CLIENT = client.Client(_use_grpc=False) # Skip the test cases using bigquery, storage and pubsub clients for mTLS testing. @@ -261,12 +263,15 @@ def test_list_entry_with_requestlog(self): def test_log_text(self): TEXT_PAYLOAD = "System test: test_log_text" - logger = Config.CLIENT.logger(self._logger_name("log_text")) - self.to_delete.append(logger) - logger.log_text(TEXT_PAYLOAD) - entries = _list_entries(logger) - self.assertEqual(len(entries), 1) - self.assertEqual(entries[0].payload, TEXT_PAYLOAD) + gapic_logger = Config.CLIENT.logger(self._logger_name("log_text")) + http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_text_http")) + for logger in [gapic_logger, http_logger]: + self.to_delete.append(logger) + # test gapic + logger.log_text(TEXT_PAYLOAD) + entries = _list_entries(logger) + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0].payload, TEXT_PAYLOAD) def test_log_text_with_timestamp(self): text_payload = "System test: test_log_text_with_timestamp" From 6c5ccd28545cd89c8385d3ba7be798454b925a3d Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 3 Aug 2021 16:25:50 -0700 Subject: [PATCH 02/16] added http tests --- tests/system/test_system.py | 74 +++++++++++++------------------------ 1 file changed, 26 insertions(+), 48 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index c48f61e3c..1080dd733 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -188,34 +188,34 @@ def test_list_entry_with_auditlog(self): audit_dict = { "@type": type_url, "methodName": "test", - "requestMetadata": {"callerIp": "::1", "callerSuppliedUserAgent": "test"}, "resourceName": "test", "serviceName": "test", - "status": {"code": 0}, } audit_struct = self._dict_to_struct(audit_dict) - logger = Config.CLIENT.logger(f"audit-proto-{uuid.uuid1()}") - logger.log_proto(audit_struct) - - # retrieve log - retry = RetryErrors((TooManyRequests, StopIteration), max_tries=8) - protobuf_entry = retry(lambda: next(logger.list_entries()))() - - self.assertIsInstance(protobuf_entry, entries.ProtobufEntry) - self.assertIsNone(protobuf_entry.payload_pb) - self.assertIsInstance(protobuf_entry.payload_json, dict) - self.assertEqual(protobuf_entry.payload_json["@type"], type_url) - self.assertEqual( - protobuf_entry.payload_json["methodName"], audit_dict["methodName"] - ) - self.assertEqual( - protobuf_entry.to_api_repr()["protoPayload"]["@type"], type_url - ) - self.assertEqual( - protobuf_entry.to_api_repr()["protoPayload"]["methodName"], - audit_dict["methodName"], - ) + gapic_logger = Config.CLIENT.logger(f"audit-proto-{uuid.uuid1()}") + http_logger = Config.HTTP_CLIENT.logger(f"audit-proto-{uuid.uuid1()}-http") + for logger in [gapic_logger, http_logger]: + logger.log_proto(audit_struct) + + # retrieve log + retry = RetryErrors((TooManyRequests, StopIteration), max_tries=8) + protobuf_entry = retry(lambda: next(logger.list_entries()))() + + self.assertIsInstance(protobuf_entry, entries.ProtobufEntry) + self.assertIsNone(protobuf_entry.payload_pb) + self.assertIsInstance(protobuf_entry.payload_json, dict) + self.assertEqual(protobuf_entry.payload_json["@type"], type_url) + self.assertEqual( + protobuf_entry.payload_json["methodName"], audit_dict["methodName"] + ) + self.assertEqual( + protobuf_entry.to_api_repr()["protoPayload"]["@type"], type_url + ) + self.assertEqual( + protobuf_entry.to_api_repr()["protoPayload"]["methodName"], + audit_dict["methodName"], + ) def test_list_entry_with_requestlog(self): """ @@ -246,32 +246,10 @@ def test_list_entry_with_requestlog(self): } req_struct = self._dict_to_struct(req_dict) - logger = Config.CLIENT.logger(f"req-proto-{uuid.uuid1()}") - logger.log_proto(req_struct) - - # retrieve log - retry = RetryErrors((TooManyRequests, StopIteration), max_tries=8) - protobuf_entry = retry(lambda: next(logger.list_entries()))() - - self.assertIsInstance(protobuf_entry, entries.ProtobufEntry) - self.assertIsNone(protobuf_entry.payload_pb) - self.assertIsInstance(protobuf_entry.payload_json, dict) - self.assertEqual(protobuf_entry.payload_json["@type"], type_url) - self.assertEqual( - protobuf_entry.to_api_repr()["protoPayload"]["@type"], type_url - ) - - def test_log_text(self): - TEXT_PAYLOAD = "System test: test_log_text" - gapic_logger = Config.CLIENT.logger(self._logger_name("log_text")) - http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_text_http")) + gapic_logger = Config.CLIENT.logger(f"req-proto-{uuid.uuid1()}") + http_logger = Config.CLIENT.logger(f"req-proto-{uuid.uuid1()}-http") for logger in [gapic_logger, http_logger]: - self.to_delete.append(logger) - # test gapic - logger.log_text(TEXT_PAYLOAD) - entries = _list_entries(logger) - self.assertEqual(len(entries), 1) - self.assertEqual(entries[0].payload, TEXT_PAYLOAD) + logger.log_proto(req_struct) def test_log_text_with_timestamp(self): text_payload = "System test: test_log_text_with_timestamp" From bdad301135d737a6c8898fb35c1e44865ba2668e Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 3 Aug 2021 16:40:40 -0700 Subject: [PATCH 03/16] added more http tests --- tests/system/test_system.py | 197 +++++++++++++++++++++--------------- 1 file changed, 118 insertions(+), 79 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 1080dd733..1a987e38a 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -251,39 +251,65 @@ def test_list_entry_with_requestlog(self): for logger in [gapic_logger, http_logger]: logger.log_proto(req_struct) + # retrieve log + retry = RetryErrors((TooManyRequests, StopIteration), max_tries=8) + protobuf_entry = retry(lambda: next(logger.list_entries()))() + + self.assertIsInstance(protobuf_entry, entries.ProtobufEntry) + self.assertIsNone(protobuf_entry.payload_pb) + self.assertIsInstance(protobuf_entry.payload_json, dict) + self.assertEqual(protobuf_entry.payload_json["@type"], type_url) + self.assertEqual( + protobuf_entry.to_api_repr()["protoPayload"]["@type"], type_url + ) + + def test_log_text(self): + TEXT_PAYLOAD = "System test: test_log_text" + gapic_logger = Config.CLIENT.logger(self._logger_name("log_text")) + http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_text_http")) + for logger in [gapic_logger, http_logger]: + self.to_delete.append(logger) + # test gapic + logger.log_text(TEXT_PAYLOAD) + entries = _list_entries(logger) + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0].payload, TEXT_PAYLOAD) + def test_log_text_with_timestamp(self): text_payload = "System test: test_log_text_with_timestamp" - logger = Config.CLIENT.logger(self._logger_name("log_text_ts")) + gapic_logger = Config.CLIENT.logger(self._logger_name("log_text_ts")) + http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_text_ts_http")) now = datetime.utcnow() - - self.to_delete.append(logger) - - logger.log_text(text_payload, timestamp=now) - entries = _list_entries(logger) - self.assertEqual(len(entries), 1) - self.assertEqual(entries[0].payload, text_payload) - self.assertEqual(entries[0].timestamp, now.replace(tzinfo=UTC)) - self.assertIsInstance(entries[0].received_timestamp, datetime) + for logger in [gapic_logger, http_logger]: + self.to_delete.append(logger) + logger.log_text(text_payload, timestamp=now) + entries = _list_entries(logger) + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0].payload, text_payload) + self.assertEqual(entries[0].timestamp, now.replace(tzinfo=UTC)) + self.assertIsInstance(entries[0].received_timestamp, datetime) def test_log_text_with_resource(self): text_payload = "System test: test_log_text_with_timestamp" - logger = Config.CLIENT.logger(self._logger_name("log_text_res")) + gapic_logger = Config.CLIENT.logger(self._logger_name("log_text_res")) + http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_text_res_http")) now = datetime.utcnow() - resource = Resource( - type="gae_app", - labels={"module_id": "default", "version_id": "test", "zone": ""}, - ) + for logger in [gapic_logger, http_logger]: + resource = Resource( + type="gae_app", + labels={"module_id": "default", "version_id": "test", "zone": ""}, + ) - self.to_delete.append(logger) + self.to_delete.append(logger) - logger.log_text(text_payload, timestamp=now, resource=resource) - entries = _list_entries(logger) - self.assertEqual(len(entries), 1) - self.assertEqual(entries[0].payload, text_payload) - # project_id is output only so we don't want it in assertion - del entries[0].resource.labels["project_id"] - self.assertEqual(entries[0].resource, resource) + logger.log_text(text_payload, timestamp=now, resource=resource) + entries = _list_entries(logger) + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0].payload, text_payload) + # project_id is output only so we don't want it in assertion + del entries[0].resource.labels["project_id"] + self.assertEqual(entries[0].resource, resource) def test_log_text_w_metadata(self): TEXT_PAYLOAD = "System test: test_log_text" @@ -293,35 +319,39 @@ def test_log_text_w_metadata(self): URI = "https://api.example.com/endpoint" STATUS = 500 REQUEST = {"requestMethod": METHOD, "requestUrl": URI, "status": STATUS} - logger = Config.CLIENT.logger(self._logger_name("log_text_md")) - self.to_delete.append(logger) + gapic_logger = Config.CLIENT.logger(self._logger_name("log_text_md")) + http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_text_md_http")) + for logger in [gapic_logger, http_logger]: + self.to_delete.append(logger) - logger.log_text( - TEXT_PAYLOAD, insert_id=INSERT_ID, severity=SEVERITY, http_request=REQUEST - ) - entries = _list_entries(logger) + logger.log_text( + TEXT_PAYLOAD, insert_id=INSERT_ID, severity=SEVERITY, http_request=REQUEST + ) + entries = _list_entries(logger) - self.assertEqual(len(entries), 1) + self.assertEqual(len(entries), 1) - entry = entries[0] - self.assertEqual(entry.payload, TEXT_PAYLOAD) - self.assertEqual(entry.insert_id, INSERT_ID) - self.assertEqual(entry.severity, SEVERITY) + entry = entries[0] + self.assertEqual(entry.payload, TEXT_PAYLOAD) + self.assertEqual(entry.insert_id, INSERT_ID) + self.assertEqual(entry.severity, SEVERITY) - request = entry.http_request - self.assertEqual(request["requestMethod"], METHOD) - self.assertEqual(request["requestUrl"], URI) - self.assertEqual(request["status"], STATUS) + request = entry.http_request + self.assertEqual(request["requestMethod"], METHOD) + self.assertEqual(request["requestUrl"], URI) + self.assertEqual(request["status"], STATUS) def test_log_struct(self): - logger = Config.CLIENT.logger(self._logger_name("log_struct")) - self.to_delete.append(logger) + gapic_logger = Config.CLIENT.logger(self._logger_name("log_struct")) + http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_struct_http")) + for logger in [gapic_logger, http_logger]: + self.to_delete.append(logger) - logger.log_struct(self.JSON_PAYLOAD) - entries = _list_entries(logger) + logger.log_struct(self.JSON_PAYLOAD) + entries = _list_entries(logger) - self.assertEqual(len(entries), 1) - self.assertEqual(entries[0].payload, self.JSON_PAYLOAD) + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0].payload, self.JSON_PAYLOAD) def test_log_struct_w_metadata(self): INSERT_ID = "INSERTID" @@ -330,54 +360,63 @@ def test_log_struct_w_metadata(self): URI = "https://api.example.com/endpoint" STATUS = 500 REQUEST = {"requestMethod": METHOD, "requestUrl": URI, "status": STATUS} - logger = Config.CLIENT.logger(self._logger_name("log_struct_md")) - self.to_delete.append(logger) + gapic_logger = Config.CLIENT.logger(self._logger_name("log_struct_md")) + http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_struct_md_http")) + for logger in [gapic_logger, http_logger]: + self.to_delete.append(logger) - logger.log_struct( - self.JSON_PAYLOAD, - insert_id=INSERT_ID, - severity=SEVERITY, - http_request=REQUEST, - ) - entries = _list_entries(logger) + logger.log_struct( + self.JSON_PAYLOAD, + insert_id=INSERT_ID, + severity=SEVERITY, + http_request=REQUEST, + ) + entries = _list_entries(logger) - self.assertEqual(len(entries), 1) - self.assertEqual(entries[0].payload, self.JSON_PAYLOAD) - self.assertEqual(entries[0].insert_id, INSERT_ID) - self.assertEqual(entries[0].severity, SEVERITY) - request = entries[0].http_request - self.assertEqual(request["requestMethod"], METHOD) - self.assertEqual(request["requestUrl"], URI) - self.assertEqual(request["status"], STATUS) + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0].payload, self.JSON_PAYLOAD) + self.assertEqual(entries[0].insert_id, INSERT_ID) + self.assertEqual(entries[0].severity, SEVERITY) + request = entries[0].http_request + self.assertEqual(request["requestMethod"], METHOD) + self.assertEqual(request["requestUrl"], URI) + self.assertEqual(request["status"], STATUS) def test_log_w_text(self): TEXT_PAYLOAD = "System test: test_log_w_text" - logger = Config.CLIENT.logger(self._logger_name("log_w_text")) - self.to_delete.append(logger) - logger.log(TEXT_PAYLOAD) - entries = _list_entries(logger) - self.assertEqual(len(entries), 1) - self.assertEqual(entries[0].payload, TEXT_PAYLOAD) + gapic_logger = Config.CLIENT.logger(self._logger_name("log_w_text")) + http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_w_text")) + for logger in [gapic_logger, http_logger]: + self.to_delete.append(logger) + logger.log(TEXT_PAYLOAD) + entries = _list_entries(logger) + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0].payload, TEXT_PAYLOAD) def test_log_w_struct(self): - logger = Config.CLIENT.logger(self._logger_name("log_w_struct")) - self.to_delete.append(logger) + gapic_logger = Config.CLIENT.logger(self._logger_name("log_w_struct")) + http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_w_struct_http")) + for logger in [gapic_logger, http_logger]: + self.to_delete.append(logger) - logger.log(self.JSON_PAYLOAD) - entries = _list_entries(logger) + logger.log(self.JSON_PAYLOAD) + entries = _list_entries(logger) - self.assertEqual(len(entries), 1) - self.assertEqual(entries[0].payload, self.JSON_PAYLOAD) + self.assertEqual(len(entries), 1) + self.assertEqual(entries[0].payload, self.JSON_PAYLOAD) def test_log_empty(self): - logger = Config.CLIENT.logger(self._logger_name("log_empty")) - self.to_delete.append(logger) + gapic_logger = Config.CLIENT.logger(self._logger_name("log_empty")) + http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_empty_http")) - logger.log() - entries = _list_entries(logger) + for logger in [gapic_logger, http_logger]: + self.to_delete.append(logger) - self.assertEqual(len(entries), 1) - self.assertIsNone(entries[0].payload) + logger.log() + entries = _list_entries(logger) + + self.assertEqual(len(entries), 1) + self.assertIsNone(entries[0].payload) def test_log_handler_async(self): LOG_MESSAGE = "It was the worst of times" From c96f60863ff0caa24459bec152d4be34184576c5 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 4 Aug 2021 10:48:05 -0700 Subject: [PATCH 04/16] added consistency test --- google/cloud/logging_v2/_http.py | 7 ++++++- tests/system/test_system.py | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/google/cloud/logging_v2/_http.py b/google/cloud/logging_v2/_http.py index 68bde346a..8c4d52230 100644 --- a/google/cloud/logging_v2/_http.py +++ b/google/cloud/logging_v2/_http.py @@ -131,7 +131,12 @@ def list_entries( ) # This method uses POST to make a read-only request. iterator._HTTP_METHOD = "POST" - return iterator + + def log_entries_pager(page_iter): + for page in page_iter: + yield page + + return log_entries_pager(iterator) def write_entries( self, diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 1a987e38a..0757e7bb8 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -743,6 +743,25 @@ def test_update_sink(self): self.assertEqual(sink.filter_, UPDATED_FILTER) self.assertEqual(sink.destination, dataset_uri) + def test_api_equality_list_logs(self): + unique_id = uuid.uuid1() + gapic_logger = Config.CLIENT.logger(f"api-list-{unique_id}") + http_logger = Config.HTTP_CLIENT.logger(f"api-list-{unique_id}") + # write log + gapic_logger.log_text("test") + # read log + def retryable(): + gapic_generator = gapic_logger.list_entries() + http_generator = http_logger.list_entries() + self.assertEqual(type(gapic_generator), type(http_generator)) + gapic_list, http_list = list(gapic_generator), list(http_generator) + self.assertEqual(len(gapic_list), 1) + self.assertEqual(len(http_list), 1) + self.assertEqual(gapic_list[0].insert_id, http_list[0].insert_id) + RetryErrors( + (ServiceUnavailable, InternalServerError, AssertionError), + delay=2, backoff=2, max_tries=6 + )(retryable)() class _DeleteWrapper(object): From 172a2eefb73491e04a13028fa8d610460a4eb17b Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 4 Aug 2021 12:43:50 -0700 Subject: [PATCH 05/16] use max_results instead of paging --- google/cloud/logging_v2/_gapic.py | 15 ++++++--------- google/cloud/logging_v2/_http.py | 16 +++++----------- google/cloud/logging_v2/client.py | 15 ++++++--------- google/cloud/logging_v2/logger.py | 18 ++++++------------ 4 files changed, 23 insertions(+), 41 deletions(-) diff --git a/google/cloud/logging_v2/_gapic.py b/google/cloud/logging_v2/_gapic.py index 7a6d70650..123bd09b3 100644 --- a/google/cloud/logging_v2/_gapic.py +++ b/google/cloud/logging_v2/_gapic.py @@ -49,8 +49,7 @@ def list_entries( *, filter_=None, order_by=None, - page_size=None, - page_token=None, + max_results=None, ): """Return a page of log entry resources. @@ -69,11 +68,6 @@ def list_entries( https://cloud.google.com/logging/docs/view/advanced_filters order_by (str) One of :data:`~logging_v2.ASCENDING` or :data:`~logging_v2.DESCENDING`. - page_size (int): maximum number of entries to return, If not passed, - defaults to a value set by the API. - page_token (str): opaque marker for the next "page" of entries. If not - passed, the API will return the first page of - entries. Returns: Iterator[~logging_v2.LogEntry] @@ -84,8 +78,7 @@ def list_entries( resource_names=resource_names, filter=filter_, order_by=order_by, - page_size=page_size, - page_token=page_token, + page_size=None, ) response = self._gapic_api.list_log_entries(request=request) @@ -97,9 +90,13 @@ def list_entries( loggers = {} def log_entries_pager(page_iter): + i = 0 for page in page_iter: + if max_results is not None and i >= max_results: + break log_entry_dict = _parse_log_entry(LogEntryPB.pb(page)) yield entry_from_resource(log_entry_dict, self._client, loggers=loggers) + i += 1 return log_entries_pager(page_iter) diff --git a/google/cloud/logging_v2/_http.py b/google/cloud/logging_v2/_http.py index 8c4d52230..861e3a20f 100644 --- a/google/cloud/logging_v2/_http.py +++ b/google/cloud/logging_v2/_http.py @@ -74,8 +74,7 @@ def list_entries( *, filter_=None, order_by=None, - page_size=None, - page_token=None, + max_results=None, ): """Return a page of log entry resources. @@ -94,11 +93,6 @@ def list_entries( https://cloud.google.com/logging/docs/view/advanced_filters order_by (str) One of :data:`~logging_v2.ASCENDING` or :data:`~logging_v2.DESCENDING`. - page_size (int): maximum number of entries to return, If not passed, - defaults to a value set by the API. - page_token (str): opaque marker for the next "page" of entries. If not - passed, the API will return the first page of - entries. Returns: Iterator[~logging_v2.LogEntry] @@ -111,9 +105,6 @@ def list_entries( if order_by is not None: extra_params["orderBy"] = order_by - if page_size is not None: - extra_params["pageSize"] = page_size - path = "/entries:list" # We attach a mutable loggers dictionary so that as Logger # objects are created by entry_from_resource, they can be @@ -126,15 +117,18 @@ def list_entries( path=path, item_to_value=item_to_value, items_key="entries", - page_token=page_token, extra_params=extra_params, ) # This method uses POST to make a read-only request. iterator._HTTP_METHOD = "POST" def log_entries_pager(page_iter): + i = 0 for page in page_iter: + if max_results is not None and i >= max_results: + break yield page + i += 1 return log_entries_pager(iterator) diff --git a/google/cloud/logging_v2/client.py b/google/cloud/logging_v2/client.py index 8b92e8e8c..bf4dbdf21 100644 --- a/google/cloud/logging_v2/client.py +++ b/google/cloud/logging_v2/client.py @@ -204,8 +204,7 @@ def list_entries( resource_names=None, filter_=None, order_by=None, - page_size=None, - page_token=None, + max_results=None, ): """Return a page of log entry resources. @@ -226,11 +225,10 @@ def list_entries( https://cloud.google.com/logging/docs/view/advanced_filters order_by (str) One of :data:`~logging_v2.ASCENDING` or :data:`~logging_v2.DESCENDING`. - page_size (int): maximum number of entries to return, If not passed, - defaults to a value set by the API. - page_token (str): opaque marker for the next "page" of entries. If not - passed, the API will return the first page of - entries. + max_results (Optional[int]): + Optional. The maximum number of entries to return. + Non-positive values are ignored. Defaults + to API unlimited. Returns: Iterator[~logging_v2.LogEntry] @@ -243,8 +241,7 @@ def list_entries( resource_names=resource_names, filter_=filter_, order_by=order_by, - page_size=page_size, - page_token=page_token, + max_results=max_results, ) def sink(self, name, *, filter_=None, destination=None): diff --git a/google/cloud/logging_v2/logger.py b/google/cloud/logging_v2/logger.py index 01221fc7b..afd9a465c 100644 --- a/google/cloud/logging_v2/logger.py +++ b/google/cloud/logging_v2/logger.py @@ -266,6 +266,7 @@ def list_entries( order_by=None, page_size=None, page_token=None, + max_results=None ): """Return a page of log entries. @@ -289,16 +290,10 @@ def list_entries( By default, a 24 hour filter is applied. order_by (Optional[str]): One of :data:`~logging_v2.ASCENDING` or :data:`~logging_v2.DESCENDING`. - page_size (Optional[int]): - Optional. The maximum number of entries in each page of results - from this request. Non-positive values are ignored. Defaults - to a sensible value set by the API. - page_token (Optional[str]): - Optional. If present, return the next batch of entries, using - the value, which must correspond to the ``nextPageToken`` value - returned in the previous response. Deprecated: use the ``pages`` - property of the returned iterator instead of manually passing - the token. + max_results (Optional[int]): + Optional. The maximum number of entries to return. + Non-positive values are ignored. Defaults + to unlimited. Returns: Iterator[~logging_v2.entries.LogEntry] @@ -317,8 +312,7 @@ def list_entries( resource_names=resource_names, filter_=filter_, order_by=order_by, - page_size=page_size, - page_token=page_token, + max_results=max_results, ) From ca565ed28c49155c400f6701651c6706700eb8d5 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 4 Aug 2021 16:27:57 -0700 Subject: [PATCH 06/16] added more tests --- tests/system/test_system.py | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 0757e7bb8..ae22ce3ff 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -747,20 +747,38 @@ def test_api_equality_list_logs(self): unique_id = uuid.uuid1() gapic_logger = Config.CLIENT.logger(f"api-list-{unique_id}") http_logger = Config.HTTP_CLIENT.logger(f"api-list-{unique_id}") - # write log - gapic_logger.log_text("test") - # read log + # write logs + log_count = 5 + for i in range(log_count): + gapic_logger.log_text(f"test {i}") def retryable(): - gapic_generator = gapic_logger.list_entries() - http_generator = http_logger.list_entries() + max_results = 3 + gapic_generator = gapic_logger.list_entries(max_results=max_results) + http_generator = http_logger.list_entries(max_results=max_results) + # returned objects should be consistent self.assertEqual(type(gapic_generator), type(http_generator)) gapic_list, http_list = list(gapic_generator), list(http_generator) - self.assertEqual(len(gapic_list), 1) - self.assertEqual(len(http_list), 1) + # max_results should limit the number of logs returned + self.assertEqual(len(gapic_list), max_results) + self.assertEqual(len(http_list), max_results) + # returned logs should be the same self.assertEqual(gapic_list[0].insert_id, http_list[0].insert_id) + # should return in ascending order + self.assertEqual(gapic_list[0].payload, "test 0") + # test reverse ordering + gapic_generator = gapic_logger.list_entries(max_results=max_results, order_by=google.cloud.logging_v2.DESCENDING) + http_generator = http_logger.list_entries(max_results=max_results, order_by=google.cloud.logging_v2.DESCENDING) + gapic_list, http_list = list(gapic_generator), list(http_generator) + self.assertEqual(len(gapic_list), max_results) + self.assertEqual(len(http_list), max_results) + # http and gapic results should be consistent + self.assertEqual(gapic_list[0].insert_id, http_list[0].insert_id) + # returned logs should be in descending order + self.assertEqual(gapic_list[0].payload, f"test {log_count-1}") + RetryErrors( (ServiceUnavailable, InternalServerError, AssertionError), - delay=2, backoff=2, max_tries=6 + delay=2, backoff=2, max_tries=3 )(retryable)() From c6ac7370144e0208872e8619d53ed3f28477dc2d Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 4 Aug 2021 16:58:05 -0700 Subject: [PATCH 07/16] updated other APIs to use max_length --- google/cloud/logging_v2/_gapic.py | 38 ++++++++++--------- google/cloud/logging_v2/_http.py | 62 +++++++++++++++---------------- google/cloud/logging_v2/client.py | 33 ++++++---------- google/cloud/logging_v2/logger.py | 5 +-- 4 files changed, 63 insertions(+), 75 deletions(-) diff --git a/google/cloud/logging_v2/_gapic.py b/google/cloud/logging_v2/_gapic.py index 123bd09b3..bb5bdc7ef 100644 --- a/google/cloud/logging_v2/_gapic.py +++ b/google/cloud/logging_v2/_gapic.py @@ -68,6 +68,9 @@ def list_entries( https://cloud.google.com/logging/docs/view/advanced_filters order_by (str) One of :data:`~logging_v2.ASCENDING` or :data:`~logging_v2.DESCENDING`. + max_results (Optional[int]): + Optional. The maximum number of entries to return. + Non-positive values are ignored. If None, uses API defaults. Returns: Iterator[~logging_v2.LogEntry] @@ -78,7 +81,7 @@ def list_entries( resource_names=resource_names, filter=filter_, order_by=order_by, - page_size=None, + page_size=max_results, ) response = self._gapic_api.list_log_entries(request=request) @@ -172,7 +175,7 @@ def __init__(self, gapic_api, client): self._gapic_api = gapic_api self._client = client - def list_sinks(self, parent, *, page_size=0, page_token=None): + def list_sinks(self, parent, *, max_results=None): """List sinks for the parent resource. Args: @@ -184,26 +187,25 @@ def list_sinks(self, parent, *, page_size=0, page_token=None): "organizations/[ORGANIZATION_ID]" "billingAccounts/[BILLING_ACCOUNT_ID]" "folders/[FOLDER_ID]". - page_size (Optional[int]): Maximum number of sinks to return, If not passed, - defaults to a value set by the API. - page_token (Optional[str]): Opaque marker for the next "page" of sinks. If not - passed, the API will return the first page of - sinks. + max_results (Optional[int]): + Optional. The maximum number of entries to return. + Non-positive values are ignored. If None, uses API defaults. Returns: Iterator[~logging_v2.Sink] """ - request = ListSinksRequest( - parent=parent, page_size=page_size, page_token=page_token - ) + request = ListSinksRequest(parent=parent) response = self._gapic_api.list_sinks(request) page_iter = iter(response) def sinks_pager(page_iter): + i = 0 for page in page_iter: + if max_results is not None and i >= max_results: + break # Convert the GAPIC sink type into the handwritten `Sink` type yield Sink.from_api_repr(LogSink.to_dict(page), client=self._client) - + i += 1 return sinks_pager(page_iter) def sink_create( @@ -344,16 +346,14 @@ def __init__(self, gapic_api, client): self._gapic_api = gapic_api self._client = client - def list_metrics(self, project, *, page_size=0, page_token=None): + def list_metrics(self, project, *, max_results=None): """List metrics for the project associated with this client. Args: project (str): ID of the project whose metrics are to be listed. - page_size (int): Maximum number of metrics to return, If not passed, - defaults to a value set by the API. - page_token (str): Opaque marker for the next "page" of metrics. If not - passed, the API will return the first page of - sinks. + max_results (Optional[int]): + Optional. The maximum number of entries to return. + Non-positive values are ignored. If None, uses API defaults. Returns: Iterable[logging_v2.Metric]: Iterable of metrics. @@ -366,9 +366,13 @@ def list_metrics(self, project, *, page_size=0, page_token=None): page_iter = iter(response) def metrics_pager(page_iter): + i = 0 for page in page_iter: + if max_results is not None and i >= max_results: + break # Convert GAPIC metrics type into handwritten `Metric` type yield Metric.from_api_repr(LogMetric.to_dict(page), client=self._client) + i += 1 return metrics_pager(page_iter) diff --git a/google/cloud/logging_v2/_http.py b/google/cloud/logging_v2/_http.py index 861e3a20f..119a20a1a 100644 --- a/google/cloud/logging_v2/_http.py +++ b/google/cloud/logging_v2/_http.py @@ -93,6 +93,9 @@ def list_entries( https://cloud.google.com/logging/docs/view/advanced_filters order_by (str) One of :data:`~logging_v2.ASCENDING` or :data:`~logging_v2.DESCENDING`. + max_results (Optional[int]): + Optional. The maximum number of entries to return. + Non-positive values are ignored. If None, uses API defaults. Returns: Iterator[~logging_v2.LogEntry] @@ -122,15 +125,7 @@ def list_entries( # This method uses POST to make a read-only request. iterator._HTTP_METHOD = "POST" - def log_entries_pager(page_iter): - i = 0 - for page in page_iter: - if max_results is not None and i >= max_results: - break - yield page - i += 1 - - return log_entries_pager(iterator) + return _entries_pager(iterator, max_results) def write_entries( self, @@ -218,7 +213,7 @@ def __init__(self, client): self._client = client self.api_request = client._connection.api_request - def list_sinks(self, parent, *, page_size=None, page_token=None): + def list_sinks(self, parent, *, max_results=None): """List sinks for the parent resource. See @@ -233,31 +228,31 @@ def list_sinks(self, parent, *, page_size=None, page_token=None): "organizations/[ORGANIZATION_ID]" "billingAccounts/[BILLING_ACCOUNT_ID]" "folders/[FOLDER_ID]". - page_size (Optional[int]): Maximum number of sinks to return, If not passed, - defaults to a value set by the API. - page_token (Optional[str]): Opaque marker for the next "page" of sinks. If not - passed, the API will return the first page of - sinks. + max_results (Optional[int]): + Optional. The maximum number of entries to return. + Non-positive values are ignored. If None, uses API defaults. Returns: Iterator[~logging_v2.Sink] """ extra_params = {} - if page_size is not None: - extra_params["pageSize"] = page_size + if max_results is not None: + extra_params["pageSize"] = max_results path = f"/{parent}/sinks" - return page_iterator.HTTPIterator( + iterator = page_iterator.HTTPIterator( client=self._client, api_request=self._client._connection.api_request, path=path, item_to_value=_item_to_sink, items_key="sinks", - page_token=page_token, extra_params=extra_params, ) + return _entries_pager(iterator, max_results) + + def sink_create( self, parent, sink_name, filter_, destination, *, unique_writer_identity=False ): @@ -372,40 +367,36 @@ def __init__(self, client): self._client = client self.api_request = client._connection.api_request - def list_metrics(self, project, *, page_size=None, page_token=None): + def list_metrics(self, project, *, max_results=None): """List metrics for the project associated with this client. See https://cloud.google.com/logging/docs/reference/v2/rest/v2/projects.metrics/list Args: - page_size (Optional[int]): The maximum number of sinks in each - page of results from this request. Non-positive values are ignored. Defaults to a - sensible value set by the API. - page_token (Optional[str]): If present, return the next batch of sinks, using the - value, which must correspond to the ``nextPageToken`` value - returned in the previous response. Deprecated: use the ``pages`` - property ofthe returned iterator instead of manually passing the - token. + max_results (Optional[int]): + Optional. The maximum number of entries to return. + Non-positive values are ignored. If None, uses API defaults. Returns: Iterator[google.cloud.logging_v2.metric.Metric] """ extra_params = {} - if page_size is not None: - extra_params["pageSize"] = page_size + if max_results is not None: + extra_params["pageSize"] = max_results path = f"/projects/{project}/metrics" - return page_iterator.HTTPIterator( + iterator = page_iterator.HTTPIterator( client=self._client, api_request=self._client._connection.api_request, path=path, item_to_value=_item_to_metric, items_key="metrics", - page_token=page_token, extra_params=extra_params, ) + return _entries_pager(iterator, max_results) + def metric_create(self, project, metric_name, filter_, description): """Create a metric resource. @@ -467,6 +458,13 @@ def metric_delete(self, project, metric_name): target = f"/projects/{project}/metrics/{metric_name}" self.api_request(method="DELETE", path=target) +def _entries_pager(page_iter, limit): + i = 0 + for page in page_iter: + if limit is not None and i >= limit: + break + yield page + i += 1 def _item_to_entry(iterator, resource, loggers): """Convert a log entry resource to the native object. diff --git a/google/cloud/logging_v2/client.py b/google/cloud/logging_v2/client.py index bf4dbdf21..b2fb7d485 100644 --- a/google/cloud/logging_v2/client.py +++ b/google/cloud/logging_v2/client.py @@ -227,8 +227,7 @@ def list_entries( or :data:`~logging_v2.DESCENDING`. max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. Defaults - to API unlimited. + Non-positive values are ignored. If None, uses API defaults. Returns: Iterator[~logging_v2.LogEntry] @@ -263,7 +262,7 @@ def sink(self, name, *, filter_=None, destination=None): """ return Sink(name, filter_=filter_, destination=destination, client=self) - def list_sinks(self, *, parent=None, page_size=None, page_token=None): + def list_sinks(self, *, parent=None, max_results=None): """List sinks for the a parent resource. See @@ -280,14 +279,9 @@ def list_sinks(self, *, parent=None, page_size=None, page_token=None): "folders/[FOLDER_ID]". If not passed, defaults to the project bound to the API's client. - page_size (Optional[int]): The maximum number of sinks in each - page of results from this request. Non-positive values are ignored. Defaults to a - sensible value set by the API. - page_token (Optional[str]): If present, return the next batch of sinks, using the - value, which must correspond to the ``nextPageToken`` value - returned in the previous response. Deprecated: use the ``pages`` - property ofthe returned iterator instead of manually passing the - token. + max_results (Optional[int]): + Optional. The maximum number of entries to return. + Non-positive values are ignored. If None, uses API defaults. Returns: Iterator[~logging_v2.sink.Sink] @@ -295,7 +289,7 @@ def list_sinks(self, *, parent=None, page_size=None, page_token=None): if parent is None: parent = f"projects/{self.project}" return self.sinks_api.list_sinks( - parent=parent, page_size=page_size, page_token=page_token + parent=parent, max_results=max_results, ) def metric(self, name, *, filter_=None, description=""): @@ -316,27 +310,22 @@ def metric(self, name, *, filter_=None, description=""): """ return Metric(name, filter_=filter_, client=self, description=description) - def list_metrics(self, *, page_size=None, page_token=None): + def list_metrics(self, *, max_results=None): """List metrics for the project associated with this client. See https://cloud.google.com/logging/docs/reference/v2/rest/v2/projects.metrics/list Args: - page_size (Optional[int]): The maximum number of sinks in each - page of results from this request. Non-positive values are ignored. Defaults to a - sensible value set by the API. - page_token (Optional[str]): If present, return the next batch of sinks, using the - value, which must correspond to the ``nextPageToken`` value - returned in the previous response. Deprecated: use the ``pages`` - property ofthe returned iterator instead of manually passing the - token. + max_results (Optional[int]): + Optional. The maximum number of entries to return. + Non-positive values are ignored. If None, uses API defaults. Returns: Iterator[~logging_v2.metric.Metric] """ return self.metrics_api.list_metrics( - self.project, page_size=page_size, page_token=page_token + self.project, max_results=max_results ) def get_default_handler(self, **kw): diff --git a/google/cloud/logging_v2/logger.py b/google/cloud/logging_v2/logger.py index afd9a465c..c5afd311f 100644 --- a/google/cloud/logging_v2/logger.py +++ b/google/cloud/logging_v2/logger.py @@ -264,8 +264,6 @@ def list_entries( resource_names=None, filter_=None, order_by=None, - page_size=None, - page_token=None, max_results=None ): """Return a page of log entries. @@ -292,8 +290,7 @@ def list_entries( or :data:`~logging_v2.DESCENDING`. max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. Defaults - to unlimited. + Non-positive values are ignored. If None, uses API defaults. Returns: Iterator[~logging_v2.entries.LogEntry] From 8640ec5faa2a1ca7eb5355157b039d66469380cd Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 13 Aug 2021 23:51:37 +0000 Subject: [PATCH 08/16] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- google/cloud/logging_v2/_gapic.py | 8 ++------ google/cloud/logging_v2/_http.py | 15 +++++---------- google/cloud/logging_v2/client.py | 15 +++------------ google/cloud/logging_v2/logger.py | 7 +------ tests/system/test_system.py | 23 +++++++++++++++++------ 5 files changed, 28 insertions(+), 40 deletions(-) diff --git a/google/cloud/logging_v2/_gapic.py b/google/cloud/logging_v2/_gapic.py index bb5bdc7ef..c4f2d4275 100644 --- a/google/cloud/logging_v2/_gapic.py +++ b/google/cloud/logging_v2/_gapic.py @@ -44,12 +44,7 @@ def __init__(self, gapic_api, client): self._client = client def list_entries( - self, - resource_names, - *, - filter_=None, - order_by=None, - max_results=None, + self, resource_names, *, filter_=None, order_by=None, max_results=None, ): """Return a page of log entry resources. @@ -206,6 +201,7 @@ def sinks_pager(page_iter): # Convert the GAPIC sink type into the handwritten `Sink` type yield Sink.from_api_repr(LogSink.to_dict(page), client=self._client) i += 1 + return sinks_pager(page_iter) def sink_create( diff --git a/google/cloud/logging_v2/_http.py b/google/cloud/logging_v2/_http.py index 119a20a1a..259acd550 100644 --- a/google/cloud/logging_v2/_http.py +++ b/google/cloud/logging_v2/_http.py @@ -69,12 +69,7 @@ def __init__(self, client): self.api_request = client._connection.api_request def list_entries( - self, - resource_names, - *, - filter_=None, - order_by=None, - max_results=None, + self, resource_names, *, filter_=None, order_by=None, max_results=None, ): """Return a page of log entry resources. @@ -241,7 +236,7 @@ def list_sinks(self, parent, *, max_results=None): extra_params["pageSize"] = max_results path = f"/{parent}/sinks" - iterator = page_iterator.HTTPIterator( + iterator = page_iterator.HTTPIterator( client=self._client, api_request=self._client._connection.api_request, path=path, @@ -252,7 +247,6 @@ def list_sinks(self, parent, *, max_results=None): return _entries_pager(iterator, max_results) - def sink_create( self, parent, sink_name, filter_, destination, *, unique_writer_identity=False ): @@ -387,7 +381,7 @@ def list_metrics(self, project, *, max_results=None): extra_params["pageSize"] = max_results path = f"/projects/{project}/metrics" - iterator = page_iterator.HTTPIterator( + iterator = page_iterator.HTTPIterator( client=self._client, api_request=self._client._connection.api_request, path=path, @@ -397,7 +391,6 @@ def list_metrics(self, project, *, max_results=None): ) return _entries_pager(iterator, max_results) - def metric_create(self, project, metric_name, filter_, description): """Create a metric resource. @@ -458,6 +451,7 @@ def metric_delete(self, project, metric_name): target = f"/projects/{project}/metrics/{metric_name}" self.api_request(method="DELETE", path=target) + def _entries_pager(page_iter, limit): i = 0 for page in page_iter: @@ -466,6 +460,7 @@ def _entries_pager(page_iter, limit): yield page i += 1 + def _item_to_entry(iterator, resource, loggers): """Convert a log entry resource to the native object. diff --git a/google/cloud/logging_v2/client.py b/google/cloud/logging_v2/client.py index b2fb7d485..b1c8854cd 100644 --- a/google/cloud/logging_v2/client.py +++ b/google/cloud/logging_v2/client.py @@ -199,12 +199,7 @@ def logger(self, name, *, labels=None, resource=None): return Logger(name, client=self, labels=labels, resource=resource) def list_entries( - self, - *, - resource_names=None, - filter_=None, - order_by=None, - max_results=None, + self, *, resource_names=None, filter_=None, order_by=None, max_results=None, ): """Return a page of log entry resources. @@ -288,9 +283,7 @@ def list_sinks(self, *, parent=None, max_results=None): """ if parent is None: parent = f"projects/{self.project}" - return self.sinks_api.list_sinks( - parent=parent, max_results=max_results, - ) + return self.sinks_api.list_sinks(parent=parent, max_results=max_results,) def metric(self, name, *, filter_=None, description=""): """Creates a metric bound to the current client. @@ -324,9 +317,7 @@ def list_metrics(self, *, max_results=None): Returns: Iterator[~logging_v2.metric.Metric] """ - return self.metrics_api.list_metrics( - self.project, max_results=max_results - ) + return self.metrics_api.list_metrics(self.project, max_results=max_results) def get_default_handler(self, **kw): """Return the default logging handler based on the local environment. diff --git a/google/cloud/logging_v2/logger.py b/google/cloud/logging_v2/logger.py index c5afd311f..5819bbb7d 100644 --- a/google/cloud/logging_v2/logger.py +++ b/google/cloud/logging_v2/logger.py @@ -259,12 +259,7 @@ def delete(self, logger_name=None, *, client=None): client.logging_api.logger_delete(logger_name) def list_entries( - self, - *, - resource_names=None, - filter_=None, - order_by=None, - max_results=None + self, *, resource_names=None, filter_=None, order_by=None, max_results=None ): """Return a page of log entries. diff --git a/tests/system/test_system.py b/tests/system/test_system.py index ae22ce3ff..bba620c80 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -103,7 +103,7 @@ class Config(object): def setUpModule(): Config.CLIENT = client.Client() - Config.HTTP_CLIENT = client.Client(_use_grpc=False) + Config.HTTP_CLIENT = client.Client(_use_grpc=False) # Skip the test cases using bigquery, storage and pubsub clients for mTLS testing. @@ -325,7 +325,10 @@ def test_log_text_w_metadata(self): self.to_delete.append(logger) logger.log_text( - TEXT_PAYLOAD, insert_id=INSERT_ID, severity=SEVERITY, http_request=REQUEST + TEXT_PAYLOAD, + insert_id=INSERT_ID, + severity=SEVERITY, + http_request=REQUEST, ) entries = _list_entries(logger) @@ -743,6 +746,7 @@ def test_update_sink(self): self.assertEqual(sink.filter_, UPDATED_FILTER) self.assertEqual(sink.destination, dataset_uri) + def test_api_equality_list_logs(self): unique_id = uuid.uuid1() gapic_logger = Config.CLIENT.logger(f"api-list-{unique_id}") @@ -751,6 +755,7 @@ def test_api_equality_list_logs(self): log_count = 5 for i in range(log_count): gapic_logger.log_text(f"test {i}") + def retryable(): max_results = 3 gapic_generator = gapic_logger.list_entries(max_results=max_results) @@ -766,8 +771,12 @@ def retryable(): # should return in ascending order self.assertEqual(gapic_list[0].payload, "test 0") # test reverse ordering - gapic_generator = gapic_logger.list_entries(max_results=max_results, order_by=google.cloud.logging_v2.DESCENDING) - http_generator = http_logger.list_entries(max_results=max_results, order_by=google.cloud.logging_v2.DESCENDING) + gapic_generator = gapic_logger.list_entries( + max_results=max_results, order_by=google.cloud.logging_v2.DESCENDING + ) + http_generator = http_logger.list_entries( + max_results=max_results, order_by=google.cloud.logging_v2.DESCENDING + ) gapic_list, http_list = list(gapic_generator), list(http_generator) self.assertEqual(len(gapic_list), max_results) self.assertEqual(len(http_list), max_results) @@ -777,8 +786,10 @@ def retryable(): self.assertEqual(gapic_list[0].payload, f"test {log_count-1}") RetryErrors( - (ServiceUnavailable, InternalServerError, AssertionError), - delay=2, backoff=2, max_tries=3 + (ServiceUnavailable, InternalServerError, AssertionError), + delay=2, + backoff=2, + max_tries=3, )(retryable)() From 1210136eaf34b5c4cf6cbef8d7910116cbc693ac Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 17 Sep 2021 16:35:34 -0700 Subject: [PATCH 09/16] fixed system tests --- google/cloud/logging_v2/_gapic.py | 2 +- tests/system/test_system.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/google/cloud/logging_v2/_gapic.py b/google/cloud/logging_v2/_gapic.py index c4f2d4275..ba00a1ee6 100644 --- a/google/cloud/logging_v2/_gapic.py +++ b/google/cloud/logging_v2/_gapic.py @@ -356,7 +356,7 @@ def list_metrics(self, project, *, max_results=None): """ path = f"projects/{project}" request = ListLogMetricsRequest( - parent=path, page_size=page_size, page_token=page_token, + parent=path, page_size=max_results, ) response = self._gapic_api.list_log_metrics(request=request) page_iter = iter(response) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index bba620c80..271c32c62 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -36,6 +36,10 @@ from google.cloud.logging_v2.handlers.transports import SyncTransport from google.cloud.logging_v2 import client from google.cloud.logging_v2.resource import Resource +from google.cloud.logging_v2.entries import LogEntry +from google.cloud.logging_v2.entries import ProtobufEntry +from google.cloud.logging_v2.entries import StructEntry +from google.cloud.logging_v2.entries import TextEntry from google.protobuf.struct_pb2 import Struct, Value, ListValue, NullValue @@ -269,11 +273,11 @@ def test_log_text(self): http_logger = Config.HTTP_CLIENT.logger(self._logger_name("log_text_http")) for logger in [gapic_logger, http_logger]: self.to_delete.append(logger) - # test gapic logger.log_text(TEXT_PAYLOAD) entries = _list_entries(logger) self.assertEqual(len(entries), 1) self.assertEqual(entries[0].payload, TEXT_PAYLOAD) + self.assertTrue(isinstance(entries[0], TextEntry)) def test_log_text_with_timestamp(self): text_payload = "System test: test_log_text_with_timestamp" From f93174b1f2e4aba1a4c665a9fb2f8cbb85d2a9e0 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Mon, 20 Sep 2021 16:29:40 -0700 Subject: [PATCH 10/16] added back page_token and page_size --- google/cloud/logging_v2/_gapic.py | 97 ++++++++++++++++++++++--------- google/cloud/logging_v2/_http.py | 61 ++++++++++++++----- google/cloud/logging_v2/client.py | 61 +++++++++++++++---- google/cloud/logging_v2/logger.py | 23 ++++++-- 4 files changed, 183 insertions(+), 59 deletions(-) diff --git a/google/cloud/logging_v2/_gapic.py b/google/cloud/logging_v2/_gapic.py index ba00a1ee6..a8106e3c0 100644 --- a/google/cloud/logging_v2/_gapic.py +++ b/google/cloud/logging_v2/_gapic.py @@ -44,9 +44,16 @@ def __init__(self, gapic_api, client): self._client = client def list_entries( - self, resource_names, *, filter_=None, order_by=None, max_results=None, + self, + resource_names, + *, + filter_=None, + order_by=None, + max_results=None, + page_size=None, + page_token=None, ): - """Return a page of log entry resources. + """Return a generator of log entry resources. Args: resource_names (Sequence[str]): Names of one or more parent resources @@ -65,10 +72,14 @@ def list_entries( or :data:`~logging_v2.DESCENDING`. max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. If None, uses API defaults. - + Non-positive values are treated as 0. If None, uses API defaults. + page_size (int): number of entries to fetch in each API call. Although + requests are paged internally, logs are returned by the generator + one at a time. If not passed, defaults to a value set by the API. + page_token (str): opaque marker for the starting "page" of entries. If not + passed, the API will return the first page of entries. Returns: - Iterator[~logging_v2.LogEntry] + Generator[~logging_v2.LogEntry] """ # full resource names are expected by the API resource_names = resource_names @@ -76,27 +87,33 @@ def list_entries( resource_names=resource_names, filter=filter_, order_by=order_by, - page_size=max_results, + page_size=page_size, + page_token=page_token, ) response = self._gapic_api.list_log_entries(request=request) - page_iter = iter(response) + log_iter = iter(response) # We attach a mutable loggers dictionary so that as Logger # objects are created by entry_from_resource, they can be # re-used by other log entries from the same logger. loggers = {} - def log_entries_pager(page_iter): + if max_results is not None: + # drop negative values + max_results = max(max_results, 0) + + # create generator + def log_entries_pager(log_iter): i = 0 - for page in page_iter: + for entry in log_iter: if max_results is not None and i >= max_results: break - log_entry_dict = _parse_log_entry(LogEntryPB.pb(page)) + log_entry_dict = _parse_log_entry(LogEntryPB.pb(entry)) yield entry_from_resource(log_entry_dict, self._client, loggers=loggers) i += 1 - return log_entries_pager(page_iter) + return log_entries_pager(log_iter) def write_entries( self, @@ -170,7 +187,7 @@ def __init__(self, gapic_api, client): self._gapic_api = gapic_api self._client = client - def list_sinks(self, parent, *, max_results=None): + def list_sinks(self, parent, *, max_results=None, page_size=None, page_token=None): """List sinks for the parent resource. Args: @@ -184,25 +201,36 @@ def list_sinks(self, parent, *, max_results=None): "folders/[FOLDER_ID]". max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. If None, uses API defaults. + Non-positive values are treated as 0. If None, uses API defaults. + page_size (int): number of entries to fetch in each API call. Although + requests are paged internally, logs are returned by the generator + one at a time. If not passed, defaults to a value set by the API. + page_token (str): opaque marker for the starting "page" of entries. If not + passed, the API will return the first page of entries. Returns: - Iterator[~logging_v2.Sink] + Generator[~logging_v2.Sink] """ - request = ListSinksRequest(parent=parent) + request = ListSinksRequest( + parent=parent, page_size=page_size, page_token=page_token + ) response = self._gapic_api.list_sinks(request) - page_iter = iter(response) + sink_iter = iter(response) + + if max_results is not None: + # drop negative values + max_results = max(max_results, 0) - def sinks_pager(page_iter): + def sinks_pager(sink_iter): i = 0 - for page in page_iter: + for entry in sink_iter: if max_results is not None and i >= max_results: break # Convert the GAPIC sink type into the handwritten `Sink` type - yield Sink.from_api_repr(LogSink.to_dict(page), client=self._client) + yield Sink.from_api_repr(LogSink.to_dict(entry), client=self._client) i += 1 - return sinks_pager(page_iter) + return sinks_pager(sink_iter) def sink_create( self, parent, sink_name, filter_, destination, *, unique_writer_identity=False @@ -342,35 +370,48 @@ def __init__(self, gapic_api, client): self._gapic_api = gapic_api self._client = client - def list_metrics(self, project, *, max_results=None): + def list_metrics( + self, project, *, max_results=None, page_size=None, page_token=None + ): """List metrics for the project associated with this client. Args: project (str): ID of the project whose metrics are to be listed. max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. If None, uses API defaults. + Non-positive values are treated as 0. If None, uses API defaults. + page_size (int): number of entries to fetch in each API call. Although + requests are paged internally, logs are returned by the generator + one at a time. If not passed, defaults to a value set by the API. + page_token (str): opaque marker for the starting "page" of entries. If not + passed, the API will return the first page of entries. Returns: Iterable[logging_v2.Metric]: Iterable of metrics. """ path = f"projects/{project}" request = ListLogMetricsRequest( - parent=path, page_size=max_results, + parent=path, page_size=page_size, page_token=page_token, ) response = self._gapic_api.list_log_metrics(request=request) - page_iter = iter(response) + metric_iter = iter(response) + + if max_results is not None: + # drop negative values + max_results = max(max_results, 0) - def metrics_pager(page_iter): + def metrics_pager(metric_iter): i = 0 - for page in page_iter: + for entry in metric_iter: if max_results is not None and i >= max_results: break # Convert GAPIC metrics type into handwritten `Metric` type - yield Metric.from_api_repr(LogMetric.to_dict(page), client=self._client) + yield Metric.from_api_repr( + LogMetric.to_dict(entry), client=self._client + ) i += 1 - return metrics_pager(page_iter) + return metrics_pager(metric_iter) def metric_create(self, project, metric_name, filter_, description): """Create a metric resource. diff --git a/google/cloud/logging_v2/_http.py b/google/cloud/logging_v2/_http.py index 259acd550..0e3cffa29 100644 --- a/google/cloud/logging_v2/_http.py +++ b/google/cloud/logging_v2/_http.py @@ -69,7 +69,14 @@ def __init__(self, client): self.api_request = client._connection.api_request def list_entries( - self, resource_names, *, filter_=None, order_by=None, max_results=None, + self, + resource_names, + *, + filter_=None, + order_by=None, + max_results=None, + page_size=None, + page_token=None, ): """Return a page of log entry resources. @@ -90,10 +97,14 @@ def list_entries( or :data:`~logging_v2.DESCENDING`. max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. If None, uses API defaults. - + Non-positive values are treated as 0. If None, uses API defaults. + page_size (int): number of entries to fetch in each API call. Although + requests are paged internally, logs are returned by the generator + one at a time. If not passed, defaults to a value set by the API. + page_token (str): opaque marker for the starting "page" of entries. If not + passed, the API will return the first page of entries. Returns: - Iterator[~logging_v2.LogEntry] + Generator[~logging_v2.LogEntry] """ extra_params = {"resourceNames": resource_names} @@ -103,6 +114,9 @@ def list_entries( if order_by is not None: extra_params["orderBy"] = order_by + if page_size is not None: + extra_params["pageSize"] = page_size + path = "/entries:list" # We attach a mutable loggers dictionary so that as Logger # objects are created by entry_from_resource, they can be @@ -115,6 +129,7 @@ def list_entries( path=path, item_to_value=item_to_value, items_key="entries", + page_token=page_token, extra_params=extra_params, ) # This method uses POST to make a read-only request. @@ -208,7 +223,7 @@ def __init__(self, client): self._client = client self.api_request = client._connection.api_request - def list_sinks(self, parent, *, max_results=None): + def list_sinks(self, parent, *, max_results=None, page_size=None, page_token=None): """List sinks for the parent resource. See @@ -225,15 +240,20 @@ def list_sinks(self, parent, *, max_results=None): "folders/[FOLDER_ID]". max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. If None, uses API defaults. + Non-positive values are treated as 0. If None, uses API defaults. + page_size (int): number of entries to fetch in each API call. Although + requests are paged internally, logs are returned by the generator + one at a time. If not passed, defaults to a value set by the API. + page_token (str): opaque marker for the starting "page" of entries. If not + passed, the API will return the first page of entries. Returns: - Iterator[~logging_v2.Sink] + Generator[~logging_v2.Sink] """ extra_params = {} - if max_results is not None: - extra_params["pageSize"] = max_results + if page_size is not None: + extra_params["pageSize"] = page_size path = f"/{parent}/sinks" iterator = page_iterator.HTTPIterator( @@ -242,6 +262,7 @@ def list_sinks(self, parent, *, max_results=None): path=path, item_to_value=_item_to_sink, items_key="sinks", + page_token=page_token, extra_params=extra_params, ) @@ -361,7 +382,9 @@ def __init__(self, client): self._client = client self.api_request = client._connection.api_request - def list_metrics(self, project, *, max_results=None): + def list_metrics( + self, project, *, max_results=None, page_size=None, page_token=None + ): """List metrics for the project associated with this client. See @@ -370,15 +393,21 @@ def list_metrics(self, project, *, max_results=None): Args: max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. If None, uses API defaults. + Non-positive values are treated as 0. If None, uses API defaults. + page_size (int): number of entries to fetch in each API call. Although + requests are paged internally, logs are returned by the generator + one at a time. If not passed, defaults to a value set by the API. + page_token (str): opaque marker for the starting "page" of entries. If not + passed, the API will return the first page of entries. Returns: - Iterator[google.cloud.logging_v2.metric.Metric] + Iterable[logging_v2.Metric]: Iterable of metrics. + """ extra_params = {} - if max_results is not None: - extra_params["pageSize"] = max_results + if page_size is not None: + extra_params["pageSize"] = page_size path = f"/projects/{project}/metrics" iterator = page_iterator.HTTPIterator( @@ -387,6 +416,7 @@ def list_metrics(self, project, *, max_results=None): path=path, item_to_value=_item_to_metric, items_key="metrics", + page_token=page_token, extra_params=extra_params, ) return _entries_pager(iterator, max_results) @@ -453,6 +483,9 @@ def metric_delete(self, project, metric_name): def _entries_pager(page_iter, limit): + if limit is not None: + # drop negative values + limit = max(limit, 0) i = 0 for page in page_iter: if limit is not None and i >= limit: diff --git a/google/cloud/logging_v2/client.py b/google/cloud/logging_v2/client.py index b1c8854cd..039d6b77c 100644 --- a/google/cloud/logging_v2/client.py +++ b/google/cloud/logging_v2/client.py @@ -199,9 +199,16 @@ def logger(self, name, *, labels=None, resource=None): return Logger(name, client=self, labels=labels, resource=resource) def list_entries( - self, *, resource_names=None, filter_=None, order_by=None, max_results=None, + self, + *, + resource_names=None, + filter_=None, + order_by=None, + max_results=None, + page_size=None, + page_token=None, ): - """Return a page of log entry resources. + """Return a generator of log entry resources. Args: resource_names (Sequence[str]): Names of one or more parent resources @@ -222,10 +229,15 @@ def list_entries( or :data:`~logging_v2.DESCENDING`. max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. If None, uses API defaults. + Non-positive values are treated as 0. If None, uses API defaults. + page_size (int): number of entries to fetch in each API call. Although + requests are paged internally, logs are returned by the generator + one at a time. If not passed, defaults to a value set by the API. + page_token (str): opaque marker for the starting "page" of entries. If not + passed, the API will return the first page of entries. Returns: - Iterator[~logging_v2.LogEntry] + Generator[~logging_v2.LogEntry] """ if resource_names is None: resource_names = [f"projects/{self.project}"] @@ -236,6 +248,8 @@ def list_entries( filter_=filter_, order_by=order_by, max_results=max_results, + page_size=page_size, + page_token=page_token, ) def sink(self, name, *, filter_=None, destination=None): @@ -257,7 +271,9 @@ def sink(self, name, *, filter_=None, destination=None): """ return Sink(name, filter_=filter_, destination=destination, client=self) - def list_sinks(self, *, parent=None, max_results=None): + def list_sinks( + self, *, parent=None, max_results=None, page_size=None, page_token=None + ): """List sinks for the a parent resource. See @@ -276,14 +292,25 @@ def list_sinks(self, *, parent=None, max_results=None): If not passed, defaults to the project bound to the API's client. max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. If None, uses API defaults. + Non-positive values are treated as 0. If None, uses API defaults. + page_size (int): number of entries to fetch in each API call. Although + requests are paged internally, logs are returned by the generator + one at a time. If not passed, defaults to a value set by the API. + page_token (str): opaque marker for the starting "page" of entries. If not + passed, the API will return the first page of entries. + Returns: - Iterator[~logging_v2.sink.Sink] + Generator[~logging_v2.Sink] """ if parent is None: parent = f"projects/{self.project}" - return self.sinks_api.list_sinks(parent=parent, max_results=max_results,) + return self.sinks_api.list_sinks( + parent=parent, + max_results=max_results, + page_size=page_size, + page_token=page_token, + ) def metric(self, name, *, filter_=None, description=""): """Creates a metric bound to the current client. @@ -303,7 +330,7 @@ def metric(self, name, *, filter_=None, description=""): """ return Metric(name, filter_=filter_, client=self, description=description) - def list_metrics(self, *, max_results=None): + def list_metrics(self, *, max_results=None, page_size=None, page_token=None): """List metrics for the project associated with this client. See @@ -312,12 +339,22 @@ def list_metrics(self, *, max_results=None): Args: max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. If None, uses API defaults. + Non-positive values are treated as 0. If None, uses API defaults. + page_size (int): number of entries to fetch in each API call. Although + requests are paged internally, logs are returned by the generator + one at a time. If not passed, defaults to a value set by the API. + page_token (str): opaque marker for the starting "page" of entries. If not + passed, the API will return the first page of entries. Returns: - Iterator[~logging_v2.metric.Metric] + Generator[logging_v2.Metric]: Iterable of metrics. """ - return self.metrics_api.list_metrics(self.project, max_results=max_results) + return self.metrics_api.list_metrics( + self.project, + max_results=max_results, + page_size=page_size, + page_token=page_token, + ) def get_default_handler(self, **kw): """Return the default logging handler based on the local environment. diff --git a/google/cloud/logging_v2/logger.py b/google/cloud/logging_v2/logger.py index 5819bbb7d..404871bef 100644 --- a/google/cloud/logging_v2/logger.py +++ b/google/cloud/logging_v2/logger.py @@ -259,9 +259,16 @@ def delete(self, logger_name=None, *, client=None): client.logging_api.logger_delete(logger_name) def list_entries( - self, *, resource_names=None, filter_=None, order_by=None, max_results=None + self, + *, + resource_names=None, + filter_=None, + order_by=None, + max_results=None, + page_size=None, + page_token=None, ): - """Return a page of log entries. + """Return a generator of log entry resources. See https://cloud.google.com/logging/docs/reference/v2/rest/v2/entries/list @@ -285,10 +292,14 @@ def list_entries( or :data:`~logging_v2.DESCENDING`. max_results (Optional[int]): Optional. The maximum number of entries to return. - Non-positive values are ignored. If None, uses API defaults. - + Non-positive values are treated as 0. If None, uses API defaults. + page_size (int): number of entries to fetch in each API call. Although + requests are paged internally, logs are returned by the generator + one at a time. If not passed, defaults to a value set by the API. + page_token (str): opaque marker for the starting "page" of entries. If not + passed, the API will return the first page of entries. Returns: - Iterator[~logging_v2.entries.LogEntry] + Generator[~logging_v2.LogEntry] """ if resource_names is None: @@ -305,6 +316,8 @@ def list_entries( filter_=filter_, order_by=order_by, max_results=max_results, + page_size=page_size, + page_token=page_token, ) From bdcf77740184d1fbf620a92632a5bd80e2ec8c70 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Mon, 20 Sep 2021 16:34:31 -0700 Subject: [PATCH 11/16] fixed comments --- google/cloud/logging_v2/_gapic.py | 2 +- google/cloud/logging_v2/_http.py | 2 +- google/cloud/logging_v2/client.py | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/google/cloud/logging_v2/_gapic.py b/google/cloud/logging_v2/_gapic.py index a8106e3c0..75e5590c0 100644 --- a/google/cloud/logging_v2/_gapic.py +++ b/google/cloud/logging_v2/_gapic.py @@ -387,7 +387,7 @@ def list_metrics( passed, the API will return the first page of entries. Returns: - Iterable[logging_v2.Metric]: Iterable of metrics. + Generator[logging_v2.Metric] """ path = f"projects/{project}" request = ListLogMetricsRequest( diff --git a/google/cloud/logging_v2/_http.py b/google/cloud/logging_v2/_http.py index 0e3cffa29..2b2667f66 100644 --- a/google/cloud/logging_v2/_http.py +++ b/google/cloud/logging_v2/_http.py @@ -401,7 +401,7 @@ def list_metrics( passed, the API will return the first page of entries. Returns: - Iterable[logging_v2.Metric]: Iterable of metrics. + Generator[logging_v2.Metric] """ extra_params = {} diff --git a/google/cloud/logging_v2/client.py b/google/cloud/logging_v2/client.py index 039d6b77c..0dbc85a26 100644 --- a/google/cloud/logging_v2/client.py +++ b/google/cloud/logging_v2/client.py @@ -299,7 +299,6 @@ def list_sinks( page_token (str): opaque marker for the starting "page" of entries. If not passed, the API will return the first page of entries. - Returns: Generator[~logging_v2.Sink] """ @@ -347,7 +346,7 @@ def list_metrics(self, *, max_results=None, page_size=None, page_token=None): passed, the API will return the first page of entries. Returns: - Generator[logging_v2.Metric]: Iterable of metrics. + Generator[logging_v2.Metric] """ return self.metrics_api.list_metrics( self.project, From 1b24f8774015767284318732a1afade2066ae718 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 21 Sep 2021 16:54:56 -0700 Subject: [PATCH 12/16] fixed unit tests --- tests/unit/test__http.py | 135 ++++++++++++++++++++++++-------------- tests/unit/test_client.py | 72 ++++++++------------ tests/unit/test_logger.py | 104 ++++++++++++++++++++++++++--- 3 files changed, 208 insertions(+), 103 deletions(-) diff --git a/tests/unit/test__http.py b/tests/unit/test__http.py index e927f6c15..3ddcdce23 100644 --- a/tests/unit/test__http.py +++ b/tests/unit/test__http.py @@ -129,16 +129,21 @@ def _make_timestamp(): NOW = datetime.datetime.utcnow().replace(tzinfo=UTC) return NOW, _datetime_to_rfc3339_w_nanos(NOW) - def test_list_entries_no_paging(self): + def test_list_entries_with_limits(self): from google.cloud.logging import Client from google.cloud.logging import TextEntry from google.cloud.logging import Logger NOW, TIMESTAMP = self._make_timestamp() IID = "IID" + IID1 = "IID1" + IID2 = "IID2" TEXT = "TEXT" SENT = {"resourceNames": [self.PROJECT_PATH]} TOKEN = "TOKEN" + PAYLOAD = {"message": "MESSAGE", "weather": "partly cloudy"} + PROTO_PAYLOAD = PAYLOAD.copy() + PROTO_PAYLOAD["@type"] = "type.googleapis.com/testing.example" RETURNED = { "entries": [ { @@ -147,24 +152,42 @@ def test_list_entries_no_paging(self): "resource": {"type": "global"}, "timestamp": TIMESTAMP, "logName": f"projects/{self.PROJECT}/logs/{self.LOGGER_NAME}", - } + }, + { + "jsonPayload": PAYLOAD, + "insertId": IID1, + "resource": {"type": "global"}, + "timestamp": TIMESTAMP, + "logName": "projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME), + }, + { + "protoPayload": PROTO_PAYLOAD, + "insertId": IID2, + "resource": {"type": "global"}, + "timestamp": TIMESTAMP, + "logName": "projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME), + }, ], - "nextPageToken": TOKEN, } client = Client( project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False ) + # try with negative max_results client._connection = _Connection(RETURNED) api = self._make_one(client) - - iterator = api.list_entries([self.PROJECT_PATH]) - page = next(iterator.pages) - entries = list(page) - token = iterator.next_page_token - - # First check the token. - self.assertEqual(token, TOKEN) - # Then check the entries returned. + empty = list(api.list_entries([self.PROJECT_PATH], max_results=-1)) + self.assertEqual(empty, []) + # try with max_results of 0 + client._connection = _Connection(RETURNED) + api = self._make_one(client) + empty = list(api.list_entries([self.PROJECT_PATH], max_results=0)) + self.assertEqual(empty, []) + # try with single result + client._connection = _Connection(RETURNED) + api = self._make_one(client) + iterator = api.list_entries([self.PROJECT_PATH], max_results=1) + entries = list(iterator) + # check the entries returned. self.assertEqual(len(entries), 1) entry = entries[0] self.assertIsInstance(entry, TextEntry) @@ -183,7 +206,7 @@ def test_list_entries_no_paging(self): called_with, {"method": "POST", "path": expected_path, "data": SENT} ) - def test_list_entries_w_paging(self): + def test_list_entries(self): from google.cloud.logging import DESCENDING from google.cloud.logging import Client from google.cloud.logging import Logger @@ -241,11 +264,8 @@ def test_list_entries_w_paging(self): page_token=TOKEN, ) entries = list(iterator) - token = iterator.next_page_token - # First check the token. - self.assertIsNone(token) - # Then check the entries returned. + # Check the entries returned. self.assertEqual(len(entries), 2) entry1 = entries[0] self.assertIsInstance(entry1, StructEntry) @@ -361,7 +381,7 @@ def test_ctor(self): self.assertIs(api._client, client) self.assertEqual(api.api_request, connection.api_request) - def test_list_sinks_no_paging(self): + def test_list_sinks_max_returned(self): from google.cloud.logging import Sink TOKEN = "TOKEN" @@ -371,22 +391,33 @@ def test_list_sinks_no_paging(self): "name": self.SINK_PATH, "filter": self.FILTER, "destination": self.DESTINATION_URI, + }, + { + "name": "test", + "filter": "test", + "destination": "test", } ], - "nextPageToken": TOKEN, } + # try with negative max_results conn = _Connection(RETURNED) client = _Client(conn) api = self._make_one(client) - - iterator = api.list_sinks(self.PROJECT_PATH) - page = next(iterator.pages) - sinks = list(page) - token = iterator.next_page_token - - # First check the token. - self.assertEqual(token, TOKEN) - # Then check the sinks returned. + empty = list(api.list_sinks(self.PROJECT_PATH, max_results=-1)) + self.assertEqual(empty, []) + # try with max_results of 0 + conn = _Connection(RETURNED) + client = _Client(conn) + api = self._make_one(client) + empty = list(api.list_sinks(self.PROJECT_PATH, max_results=0)) + self.assertEqual(empty, []) + # try with single result + conn = _Connection(RETURNED) + client = _Client(conn) + api = self._make_one(client) + iterator = api.list_sinks(self.PROJECT_PATH, max_results=1) + sinks = list(iterator) + # Check the sinks returned. self.assertEqual(len(sinks), 1) sink = sinks[0] self.assertIsInstance(sink, Sink) @@ -401,7 +432,7 @@ def test_list_sinks_no_paging(self): called_with, {"method": "GET", "path": path, "query_params": {}} ) - def test_list_sinks_w_paging(self): + def test_list_sinks(self): from google.cloud.logging import Sink TOKEN = "TOKEN" @@ -423,11 +454,7 @@ def test_list_sinks_w_paging(self): self.PROJECT_PATH, page_size=PAGE_SIZE, page_token=TOKEN ) sinks = list(iterator) - token = iterator.next_page_token - - # First check the token. - self.assertIsNone(token) - # Then check the sinks returned. + # Check the sinks returned. self.assertEqual(len(sinks), 1) sink = sinks[0] self.assertIsInstance(sink, Sink) @@ -632,26 +659,36 @@ def _get_target_class(): def _make_one(self, *args, **kw): return self._get_target_class()(*args, **kw) - def test_list_metrics_no_paging(self): + def test_list_metrics_max_results(self): from google.cloud.logging import Metric TOKEN = "TOKEN" RETURNED = { - "metrics": [{"name": self.METRIC_PATH, "filter": self.FILTER}], - "nextPageToken": TOKEN, + "metrics": [ + {"name": self.METRIC_PATH, "filter": self.FILTER}, + {"name": "test", "filter": "test"}, + ], } + # try with negative max_results + conn = _Connection(RETURNED) + client = _Client(conn) + api = self._make_one(client) + empty = list(api.list_metrics(self.PROJECT, max_results=-1)) + self.assertEqual(empty, []) + # try with max_results of 0 + conn = _Connection(RETURNED) + client = _Client(conn) + api = self._make_one(client) + empty = list(api.list_metrics(self.PROJECT, max_results=0)) + self.assertEqual(empty, []) + # try with single result conn = _Connection(RETURNED) client = _Client(conn) api = self._make_one(client) - iterator = api.list_metrics(self.PROJECT) - page = next(iterator.pages) - metrics = list(page) - token = iterator.next_page_token - - # First check the token. - self.assertEqual(token, TOKEN) - # Then check the metrics returned. + iterator = api.list_metrics(self.PROJECT, max_results=1) + metrics = list(iterator) + # Check the metrics returned. self.assertEqual(len(metrics), 1) metric = metrics[0] self.assertIsInstance(metric, Metric) @@ -666,7 +703,7 @@ def test_list_metrics_no_paging(self): called_with, {"method": "GET", "path": path, "query_params": {}} ) - def test_list_metrics_w_paging(self): + def test_list_metrics(self): from google.cloud.logging import Metric TOKEN = "TOKEN" @@ -678,11 +715,7 @@ def test_list_metrics_w_paging(self): iterator = api.list_metrics(self.PROJECT, page_size=PAGE_SIZE, page_token=TOKEN) metrics = list(iterator) - token = iterator.next_page_token - - # First check the token. - self.assertIsNone(token) - # Then check the metrics returned. + # Check the metrics returned. self.assertEqual(len(metrics), 1) metric = metrics[0] self.assertIsInstance(metric, Metric) diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 46526fb21..ec0909fc2 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -259,7 +259,6 @@ def test_list_entries_defaults(self): IID = "IID" TEXT = "TEXT" - TOKEN = "TOKEN" ENTRIES = [ { "textPayload": TEXT, @@ -272,13 +271,11 @@ def test_list_entries_defaults(self): client = self._make_one( project=self.PROJECT, credentials=creds, _use_grpc=False ) - returned = {"entries": ENTRIES, "nextPageToken": TOKEN} + returned = {"entries": ENTRIES} client._connection = _Connection(returned) iterator = client.list_entries() - page = next(iterator.pages) - entries = list(page) - token = iterator.next_page_token + entries = list(iterator) self.assertEqual(len(entries), 1) entry = entries[0] @@ -289,7 +286,6 @@ def test_list_entries_defaults(self): self.assertEqual(logger.name, self.LOGGER_NAME) self.assertIs(logger.client, client) self.assertEqual(logger.project, self.PROJECT) - self.assertEqual(token, TOKEN) # check call payload call_payload_no_filter = deepcopy(client._connection._called_with) @@ -302,6 +298,7 @@ def test_list_entries_defaults(self): "data": { "filter": "removed", "resourceNames": [f"projects/{self.PROJECT}"], + }, }, ) @@ -342,6 +339,12 @@ def test_list_entries_explicit(self): "resource": {"type": "global"}, "logName": "projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME), }, + { + "protoPayload": "ignored", + "insertId": "ignored", + "resource": {"type": "global"}, + "logName": "projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME), + }, ] client = self._make_one( project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False @@ -355,13 +358,10 @@ def test_list_entries_explicit(self): order_by=DESCENDING, page_size=PAGE_SIZE, page_token=TOKEN, + max_results=2, ) entries = list(iterator) - token = iterator.next_page_token - - # First, check the token. - self.assertIsNone(token) - # Then check the entries. + # Check the entries. self.assertEqual(len(entries), 2) entry = entries[0] self.assertIsInstance(entry, StructEntry) @@ -423,7 +423,6 @@ def test_list_entries_explicit_timestamp(self): PAYLOAD = {"message": "MESSAGE", "weather": "partly cloudy"} PROTO_PAYLOAD = PAYLOAD.copy() PROTO_PAYLOAD["@type"] = "type.googleapis.com/testing.example" - TOKEN = "TOKEN" PAGE_SIZE = 42 ENTRIES = [ { @@ -450,14 +449,9 @@ def test_list_entries_explicit_timestamp(self): filter_=INPUT_FILTER, order_by=DESCENDING, page_size=PAGE_SIZE, - page_token=TOKEN, ) entries = list(iterator) - token = iterator.next_page_token - - # First, check the token. - self.assertIsNone(token) - # Then check the entries. + # Check the entries. self.assertEqual(len(entries), 2) entry = entries[0] self.assertIsInstance(entry, StructEntry) @@ -491,7 +485,6 @@ def test_list_entries_explicit_timestamp(self): "filter": INPUT_FILTER, "orderBy": DESCENDING, "pageSize": PAGE_SIZE, - "pageToken": TOKEN, "resourceNames": [f"projects/{PROJECT1}", f"projects/{PROJECT2}"], }, }, @@ -529,7 +522,6 @@ def test_list_sinks_no_paging(self): from google.cloud.logging import Sink PROJECT = "PROJECT" - TOKEN = "TOKEN" SINK_NAME = "sink_name" FILTER = "logName:syslog AND severity>=ERROR" SINKS = [ @@ -538,17 +530,13 @@ def test_list_sinks_no_paging(self): client = self._make_one( project=PROJECT, credentials=_make_credentials(), _use_grpc=False ) - returned = {"sinks": SINKS, "nextPageToken": TOKEN} + returned = {"sinks": SINKS} client._connection = _Connection(returned) iterator = client.list_sinks() - page = next(iterator.pages) - sinks = list(page) - token = iterator.next_page_token + sinks = list(iterator) - # First check the token. - self.assertEqual(token, TOKEN) - # Then check the sinks returned. + # Check the sinks returned. self.assertEqual(len(sinks), 1) sink = sinks[0] self.assertIsInstance(sink, Sink) @@ -573,7 +561,8 @@ def test_list_sinks_with_paging(self): TOKEN = "TOKEN" PAGE_SIZE = 42 SINKS = [ - {"name": SINK_NAME, "filter": FILTER, "destination": self.DESTINATION_URI} + {"name": SINK_NAME, "filter": FILTER, "destination": self.DESTINATION_URI}, + {"name": "test", "filter": "test", "destination": "test"} ] client = self._make_one( project=PROJECT, credentials=_make_credentials(), _use_grpc=False @@ -581,13 +570,9 @@ def test_list_sinks_with_paging(self): returned = {"sinks": SINKS} client._connection = _Connection(returned) - iterator = client.list_sinks(page_size=PAGE_SIZE, page_token=TOKEN) + iterator = client.list_sinks(page_size=PAGE_SIZE, page_token=TOKEN, max_results=1) sinks = list(iterator) - token = iterator.next_page_token - - # First check the token. - self.assertIsNone(token) - # Then check the sinks returned. + # Check the sinks returned. self.assertEqual(len(sinks), 1) sink = sinks[0] self.assertIsInstance(sink, Sink) @@ -678,29 +663,30 @@ def test_list_metrics_with_paging(self): from google.cloud.logging import Metric token = "TOKEN" - next_token = "T00KEN" page_size = 42 metrics = [ { "name": self.METRIC_NAME, "filter": self.FILTER, "description": self.DESCRIPTION, + }, + { + "name": "test", + "filter": "test", + "description": "test", } + ] client = self._make_one( project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False ) - returned = {"metrics": metrics, "nextPageToken": next_token} + returned = {"metrics": metrics} client._connection = _Connection(returned) # Execute request. - iterator = client.list_metrics(page_size=page_size, page_token=token) - page = next(iterator.pages) - metrics = list(page) - - # First check the token. - self.assertEqual(iterator.next_page_token, next_token) - # Then check the metrics returned. + iterator = client.list_metrics(page_size=page_size, page_token=token, max_results=1) + metrics = list(iterator) + # Check the metrics returned. self.assertEqual(len(metrics), 1) metric = metrics[0] self.assertIsInstance(metric, Metric) diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index 0d8fd1208..720d30cce 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -610,18 +610,15 @@ def test_list_entries_defaults(self): client = Client( project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False ) - returned = {"nextPageToken": TOKEN} + returned = {} client._connection = _Connection(returned) logger = self._make_one(self.LOGGER_NAME, client=client) iterator = logger.list_entries() - page = next(iterator.pages) - entries = list(page) - token = iterator.next_page_token + entries = list(iterator) self.assertEqual(len(entries), 0) - self.assertEqual(token, TOKEN) LOG_FILTER = "logName=projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME) # check call payload @@ -668,10 +665,8 @@ def test_list_entries_explicit(self): page_token=TOKEN, ) entries = list(iterator) - token = iterator.next_page_token self.assertEqual(len(entries), 0) - self.assertIsNone(token) # self.assertEqual(client._listed, LISTED) # check call payload call_payload_no_filter = deepcopy(client._connection._called_with) @@ -728,10 +723,8 @@ def test_list_entries_explicit_timestamp(self): page_token=TOKEN, ) entries = list(iterator) - token = iterator.next_page_token self.assertEqual(len(entries), 0) - self.assertIsNone(token) # self.assertEqual(client._listed, LISTED) # check call payload LOG_FILTER = "logName=projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME,) @@ -751,6 +744,99 @@ def test_list_entries_explicit_timestamp(self): }, ) + def test_list_entries_limit(self): + from google.cloud.logging import DESCENDING + from google.cloud.logging import ProtobufEntry + from google.cloud.logging import StructEntry + from google.cloud.logging import Logger + from google.cloud.logging import Client + + PROJECT1 = "PROJECT1" + PROJECT2 = "PROJECT2" + INPUT_FILTER = "logName:LOGNAME" + IID1 = "IID1" + IID2 = "IID2" + PAYLOAD = {"message": "MESSAGE", "weather": "partly cloudy"} + PROTO_PAYLOAD = PAYLOAD.copy() + PROTO_PAYLOAD["@type"] = "type.googleapis.com/testing.example" + TOKEN = "TOKEN" + PAGE_SIZE = 42 + ENTRIES = [ + { + "jsonPayload": PAYLOAD, + "insertId": IID1, + "resource": {"type": "global"}, + "logName": "projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME), + }, + { + "protoPayload": PROTO_PAYLOAD, + "insertId": IID2, + "resource": {"type": "global"}, + "logName": "projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME), + }, + { + "protoPayload": "ignored", + "insertId": "ignored", + "resource": {"type": "global"}, + "logName": "projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME), + }, + ] + client = Client( + project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False + ) + returned = {"entries": ENTRIES} + client._connection = _Connection(returned) + logger = self._make_one(self.LOGGER_NAME, client=client) + + iterator = logger.list_entries( + resource_names=[f"projects/{PROJECT1}", f"projects/{PROJECT2}"], + filter_=INPUT_FILTER, + order_by=DESCENDING, + page_size=PAGE_SIZE, + page_token=TOKEN, + max_results=2, + ) + entries = list(iterator) + # Check the entries. + self.assertEqual(len(entries), 2) + entry = entries[0] + self.assertIsInstance(entry, StructEntry) + self.assertEqual(entry.insert_id, IID1) + self.assertEqual(entry.payload, PAYLOAD) + logger = entry.logger + self.assertIsInstance(logger, Logger) + self.assertEqual(logger.name, self.LOGGER_NAME) + self.assertIs(logger.client, client) + self.assertEqual(logger.project, self.PROJECT) + + entry = entries[1] + self.assertIsInstance(entry, ProtobufEntry) + self.assertEqual(entry.insert_id, IID2) + self.assertEqual(entry.payload, PROTO_PAYLOAD) + logger = entry.logger + self.assertEqual(logger.name, self.LOGGER_NAME) + self.assertIs(logger.client, client) + self.assertEqual(logger.project, self.PROJECT) + + self.assertIs(entries[0].logger, entries[1].logger) + + # check call payload + call_payload_no_filter = deepcopy(client._connection._called_with) + call_payload_no_filter["data"]["filter"] = "removed" + self.assertEqual( + call_payload_no_filter, + { + "path": "/entries:list", + "method": "POST", + "data": { + "filter": "removed", + "orderBy": DESCENDING, + "pageSize": PAGE_SIZE, + "pageToken": TOKEN, + "resourceNames": [f"projects/{PROJECT1}", f"projects/{PROJECT2}"], + }, + }, + ) class TestBatch(unittest.TestCase): From 64dfc4b5c0b65f4cc618573097bed0a2b133fdf5 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Tue, 21 Sep 2021 23:56:32 +0000 Subject: [PATCH 13/16] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- tests/unit/test__http.py | 6 +----- tests/unit/test_client.py | 18 ++++++++---------- tests/unit/test_logger.py | 1 + 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/tests/unit/test__http.py b/tests/unit/test__http.py index 3ddcdce23..6a9b8da7c 100644 --- a/tests/unit/test__http.py +++ b/tests/unit/test__http.py @@ -392,11 +392,7 @@ def test_list_sinks_max_returned(self): "filter": self.FILTER, "destination": self.DESTINATION_URI, }, - { - "name": "test", - "filter": "test", - "destination": "test", - } + {"name": "test", "filter": "test", "destination": "test",}, ], } # try with negative max_results diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index ec0909fc2..2cbbf30dc 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -298,7 +298,6 @@ def test_list_entries_defaults(self): "data": { "filter": "removed", "resourceNames": [f"projects/{self.PROJECT}"], - }, }, ) @@ -562,7 +561,7 @@ def test_list_sinks_with_paging(self): PAGE_SIZE = 42 SINKS = [ {"name": SINK_NAME, "filter": FILTER, "destination": self.DESTINATION_URI}, - {"name": "test", "filter": "test", "destination": "test"} + {"name": "test", "filter": "test", "destination": "test"}, ] client = self._make_one( project=PROJECT, credentials=_make_credentials(), _use_grpc=False @@ -570,7 +569,9 @@ def test_list_sinks_with_paging(self): returned = {"sinks": SINKS} client._connection = _Connection(returned) - iterator = client.list_sinks(page_size=PAGE_SIZE, page_token=TOKEN, max_results=1) + iterator = client.list_sinks( + page_size=PAGE_SIZE, page_token=TOKEN, max_results=1 + ) sinks = list(iterator) # Check the sinks returned. self.assertEqual(len(sinks), 1) @@ -670,12 +671,7 @@ def test_list_metrics_with_paging(self): "filter": self.FILTER, "description": self.DESCRIPTION, }, - { - "name": "test", - "filter": "test", - "description": "test", - } - + {"name": "test", "filter": "test", "description": "test",}, ] client = self._make_one( project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False @@ -684,7 +680,9 @@ def test_list_metrics_with_paging(self): client._connection = _Connection(returned) # Execute request. - iterator = client.list_metrics(page_size=page_size, page_token=token, max_results=1) + iterator = client.list_metrics( + page_size=page_size, page_token=token, max_results=1 + ) metrics = list(iterator) # Check the metrics returned. self.assertEqual(len(metrics), 1) diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index 720d30cce..e70bf24e0 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -838,6 +838,7 @@ def test_list_entries_limit(self): }, ) + class TestBatch(unittest.TestCase): PROJECT = "test-project" From 41bb3f39f09563b472da2e3cb43a61264b474467 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 23 Sep 2021 14:52:47 -0700 Subject: [PATCH 14/16] fixed lint issue --- tests/system/test_system.py | 3 --- tests/unit/test__http.py | 5 +---- tests/unit/test_client.py | 2 +- tests/unit/test_logger.py | 2 -- 4 files changed, 2 insertions(+), 10 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 271c32c62..4f0233d74 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -36,9 +36,6 @@ from google.cloud.logging_v2.handlers.transports import SyncTransport from google.cloud.logging_v2 import client from google.cloud.logging_v2.resource import Resource -from google.cloud.logging_v2.entries import LogEntry -from google.cloud.logging_v2.entries import ProtobufEntry -from google.cloud.logging_v2.entries import StructEntry from google.cloud.logging_v2.entries import TextEntry from google.protobuf.struct_pb2 import Struct, Value, ListValue, NullValue diff --git a/tests/unit/test__http.py b/tests/unit/test__http.py index 6a9b8da7c..d4e6f8bac 100644 --- a/tests/unit/test__http.py +++ b/tests/unit/test__http.py @@ -140,7 +140,6 @@ def test_list_entries_with_limits(self): IID2 = "IID2" TEXT = "TEXT" SENT = {"resourceNames": [self.PROJECT_PATH]} - TOKEN = "TOKEN" PAYLOAD = {"message": "MESSAGE", "weather": "partly cloudy"} PROTO_PAYLOAD = PAYLOAD.copy() PROTO_PAYLOAD["@type"] = "type.googleapis.com/testing.example" @@ -384,7 +383,6 @@ def test_ctor(self): def test_list_sinks_max_returned(self): from google.cloud.logging import Sink - TOKEN = "TOKEN" RETURNED = { "sinks": [ { @@ -392,7 +390,7 @@ def test_list_sinks_max_returned(self): "filter": self.FILTER, "destination": self.DESTINATION_URI, }, - {"name": "test", "filter": "test", "destination": "test",}, + {"name": "test", "filter": "test", "destination": "test"}, ], } # try with negative max_results @@ -658,7 +656,6 @@ def _make_one(self, *args, **kw): def test_list_metrics_max_results(self): from google.cloud.logging import Metric - TOKEN = "TOKEN" RETURNED = { "metrics": [ {"name": self.METRIC_PATH, "filter": self.FILTER}, diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 2cbbf30dc..1a31e9c0c 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -671,7 +671,7 @@ def test_list_metrics_with_paging(self): "filter": self.FILTER, "description": self.DESCRIPTION, }, - {"name": "test", "filter": "test", "description": "test",}, + {"name": "test", "filter": "test", "description": "test"}, ] client = self._make_one( project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index e70bf24e0..ef13c923c 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -605,8 +605,6 @@ def test_delete_w_alternate_client(self): def test_list_entries_defaults(self): from google.cloud.logging import Client - TOKEN = "TOKEN" - client = Client( project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False ) From 576ff2945f5a9329cd77d1c7de69cd487439b4cb Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 5 Oct 2021 16:18:49 -0700 Subject: [PATCH 15/16] raise exception for negative values --- google/cloud/logging_v2/_gapic.py | 15 ++-- google/cloud/logging_v2/_http.py | 10 +-- tests/unit/test__gapic.py | 110 +++++++++++++++++++++++++++++- tests/unit/test__http.py | 28 ++++---- 4 files changed, 132 insertions(+), 31 deletions(-) diff --git a/google/cloud/logging_v2/_gapic.py b/google/cloud/logging_v2/_gapic.py index 75e5590c0..1e6cf0c41 100644 --- a/google/cloud/logging_v2/_gapic.py +++ b/google/cloud/logging_v2/_gapic.py @@ -99,9 +99,8 @@ def list_entries( # re-used by other log entries from the same logger. loggers = {} - if max_results is not None: - # drop negative values - max_results = max(max_results, 0) + if max_results is not None and max_results < 0: + raise ValueError('max_results must be positive') # create generator def log_entries_pager(log_iter): @@ -217,9 +216,8 @@ def list_sinks(self, parent, *, max_results=None, page_size=None, page_token=Non response = self._gapic_api.list_sinks(request) sink_iter = iter(response) - if max_results is not None: - # drop negative values - max_results = max(max_results, 0) + if max_results is not None and max_results < 0: + raise ValueError('max_results must be positive') def sinks_pager(sink_iter): i = 0 @@ -396,9 +394,8 @@ def list_metrics( response = self._gapic_api.list_log_metrics(request=request) metric_iter = iter(response) - if max_results is not None: - # drop negative values - max_results = max(max_results, 0) + if max_results is not None and max_results < 0: + raise ValueError('max_results must be positive') def metrics_pager(metric_iter): i = 0 diff --git a/google/cloud/logging_v2/_http.py b/google/cloud/logging_v2/_http.py index 2b2667f66..6e9e41bd5 100644 --- a/google/cloud/logging_v2/_http.py +++ b/google/cloud/logging_v2/_http.py @@ -482,13 +482,13 @@ def metric_delete(self, project, metric_name): self.api_request(method="DELETE", path=target) -def _entries_pager(page_iter, limit): - if limit is not None: - # drop negative values - limit = max(limit, 0) +def _entries_pager(page_iter, max_results=None): + if max_results is not None and max_results < 0: + raise ValueError('max_results must be positive') + i = 0 for page in page_iter: - if limit is not None and i >= limit: + if max_results is not None and i >= max_results: break yield page i += 1 diff --git a/tests/unit/test__gapic.py b/tests/unit/test__gapic.py index 5da1c7122..560c2bf6b 100644 --- a/tests/unit/test__gapic.py +++ b/tests/unit/test__gapic.py @@ -32,7 +32,7 @@ FILTER = "logName:syslog AND severity>=ERROR" -class Test_LoggingAPI(object): +class Test_LoggingAPI(unittest.TestCase): LOG_NAME = "log_name" LOG_PATH = f"projects/{PROJECT}/logs/{LOG_NAME}" @@ -107,6 +107,47 @@ def test_list_entries_with_options(self): assert request.page_size == 42 assert request.page_token == "token" + def test_list_logs_with_max_results(self): + client = self.make_logging_api() + log_entry_msg = LogEntryPB(log_name=self.LOG_PATH, text_payload="text") + + with mock.patch.object( + type(client._gapic_api.transport.list_log_entries), "__call__" + ) as call: + call.return_value = logging_v2.types.ListLogEntriesResponse(entries=[log_entry_msg, log_entry_msg]) + result = client.list_entries( + [PROJECT_PATH], + filter_=FILTER, + order_by=google.cloud.logging.ASCENDING, + page_size=42, + page_token="token", + max_results=1 + ) + + # Check the request + call.assert_called_once() + assert len(list(result)) == 1 + + def test_list_logs_negative_max_results(self): + client = self.make_logging_api() + + with self.assertRaises(ValueError): + with mock.patch.object( + type(client._gapic_api.transport.list_log_entries), "__call__" + ) as call: + call.return_value = logging_v2.types.ListLogEntriesResponse(entries=[]) + result = client.list_entries( + [PROJECT_PATH], + filter_=FILTER, + order_by=google.cloud.logging.ASCENDING, + page_size=42, + page_token="token", + max_results=-1 + ) + # Check the request + list(result) + call.assert_called_once() + def test_write_entries_single(self): client = self.make_logging_api() @@ -141,7 +182,7 @@ def test_logger_delete(self): assert call.call_args.args[0].log_name == self.LOG_PATH -class Test_SinksAPI(object): +class Test_SinksAPI(unittest.TestCase): SINK_NAME = "sink_name" PARENT_PATH = f"projects/{PROJECT}" SINK_PATH = f"projects/{PROJECT}/sinks/{SINK_NAME}" @@ -208,6 +249,38 @@ def test_list_sinks_with_options(self): assert request.page_size == 42 assert request.page_token == "token" + def test_list_sinks_with_max_results(self): + client = self.make_sinks_api() + sink_msg = LogSink( + name=self.SINK_NAME, destination=self.DESTINATION_URI, filter=FILTER + ) + + with mock.patch.object( + type(client._gapic_api.transport.list_sinks), "__call__" + ) as call: + call.return_value = logging_v2.types.ListSinksResponse(sinks=[sink_msg, sink_msg]) + result = client.list_sinks( + self.PARENT_PATH, page_size=42, page_token="token", max_results=1 + ) + # Check the request + call.assert_called_once() + assert len(list(result)) == 1 + + def test_list_sinks_negative_max_results(self): + client = self.make_sinks_api() + + with self.assertRaises(ValueError): + with mock.patch.object( + type(client._gapic_api.transport.list_sinks), "__call__" + ) as call: + call.return_value = logging_v2.types.ListSinksResponse(sinks=[]) + result = client.list_sinks( + self.PARENT_PATH, page_size=42, page_token="token", max_results=-1 + ) + # Check the request + list(result) + call.assert_called_once() + def test_sink_create(self): client = self.make_sinks_api() with mock.patch.object( @@ -315,7 +388,7 @@ def test_sink_delete(self): assert request.sink_name == self.SINK_PATH -class Test_MetricsAPI(object): +class Test_MetricsAPI(unittest.TestCase): METRIC_NAME = "metric_name" METRIC_PATH = f"projects/{PROJECT}/metrics/{METRIC_NAME}" DESCRIPTION = "Description" @@ -379,6 +452,37 @@ def test_list_metrics_options(self): assert request.page_size == 42 assert request.page_token == "token" + def test_list_metrics_with_max_results(self): + client = self.make_metrics_api() + metric = logging_v2.types.LogMetric( + name=self.METRIC_PATH, description=self.DESCRIPTION, filter=FILTER + ) + with mock.patch.object( + type(client._gapic_api.transport.list_log_metrics), "__call__" + ) as call: + call.return_value = logging_v2.types.ListLogMetricsResponse(metrics=[metric, metric]) + result = client.list_metrics( + PROJECT, page_size=42, page_token="token", max_results=1 + ) + # Check the request + call.assert_called_once() + assert len(list(result)) == 1 + + def test_list_metrics_negative_max_results(self): + client = self.make_metrics_api() + + with self.assertRaises(ValueError): + with mock.patch.object( + type(client._gapic_api.transport.list_log_metrics), "__call__" + ) as call: + call.return_value = logging_v2.types.ListLogMetricsResponse(metrics=[]) + result = client.list_metrics( + PROJECT, page_size=42, page_token="token", max_results=-1 + ) + # Check the request + list(result) + call.assert_called_once() + def test_metric_create(self): client = self.make_metrics_api() diff --git a/tests/unit/test__http.py b/tests/unit/test__http.py index d4e6f8bac..2154b6f57 100644 --- a/tests/unit/test__http.py +++ b/tests/unit/test__http.py @@ -172,10 +172,10 @@ def test_list_entries_with_limits(self): project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False ) # try with negative max_results - client._connection = _Connection(RETURNED) - api = self._make_one(client) - empty = list(api.list_entries([self.PROJECT_PATH], max_results=-1)) - self.assertEqual(empty, []) + with self.assertRaises(ValueError): + client._connection = _Connection(RETURNED) + api = self._make_one(client) + empty = list(api.list_entries([self.PROJECT_PATH], max_results=-1)) # try with max_results of 0 client._connection = _Connection(RETURNED) api = self._make_one(client) @@ -394,11 +394,11 @@ def test_list_sinks_max_returned(self): ], } # try with negative max_results - conn = _Connection(RETURNED) - client = _Client(conn) - api = self._make_one(client) - empty = list(api.list_sinks(self.PROJECT_PATH, max_results=-1)) - self.assertEqual(empty, []) + with self.assertRaises(ValueError): + conn = _Connection(RETURNED) + client = _Client(conn) + api = self._make_one(client) + empty = list(api.list_sinks(self.PROJECT_PATH, max_results=-1)) # try with max_results of 0 conn = _Connection(RETURNED) client = _Client(conn) @@ -663,11 +663,11 @@ def test_list_metrics_max_results(self): ], } # try with negative max_results - conn = _Connection(RETURNED) - client = _Client(conn) - api = self._make_one(client) - empty = list(api.list_metrics(self.PROJECT, max_results=-1)) - self.assertEqual(empty, []) + with self.assertRaises(ValueError): + conn = _Connection(RETURNED) + client = _Client(conn) + api = self._make_one(client) + empty = list(api.list_metrics(self.PROJECT, max_results=-1)) # try with max_results of 0 conn = _Connection(RETURNED) client = _Client(conn) From 002357b91e77b55710409e4f601f57cf0597e5a5 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Tue, 5 Oct 2021 23:21:02 +0000 Subject: [PATCH 16/16] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- google/cloud/logging_v2/_gapic.py | 6 +++--- google/cloud/logging_v2/_http.py | 2 +- tests/unit/test__gapic.py | 16 +++++++++++----- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/google/cloud/logging_v2/_gapic.py b/google/cloud/logging_v2/_gapic.py index 1e6cf0c41..3661d3d09 100644 --- a/google/cloud/logging_v2/_gapic.py +++ b/google/cloud/logging_v2/_gapic.py @@ -100,7 +100,7 @@ def list_entries( loggers = {} if max_results is not None and max_results < 0: - raise ValueError('max_results must be positive') + raise ValueError("max_results must be positive") # create generator def log_entries_pager(log_iter): @@ -217,7 +217,7 @@ def list_sinks(self, parent, *, max_results=None, page_size=None, page_token=Non sink_iter = iter(response) if max_results is not None and max_results < 0: - raise ValueError('max_results must be positive') + raise ValueError("max_results must be positive") def sinks_pager(sink_iter): i = 0 @@ -395,7 +395,7 @@ def list_metrics( metric_iter = iter(response) if max_results is not None and max_results < 0: - raise ValueError('max_results must be positive') + raise ValueError("max_results must be positive") def metrics_pager(metric_iter): i = 0 diff --git a/google/cloud/logging_v2/_http.py b/google/cloud/logging_v2/_http.py index 6e9e41bd5..21fb38606 100644 --- a/google/cloud/logging_v2/_http.py +++ b/google/cloud/logging_v2/_http.py @@ -484,7 +484,7 @@ def metric_delete(self, project, metric_name): def _entries_pager(page_iter, max_results=None): if max_results is not None and max_results < 0: - raise ValueError('max_results must be positive') + raise ValueError("max_results must be positive") i = 0 for page in page_iter: diff --git a/tests/unit/test__gapic.py b/tests/unit/test__gapic.py index 560c2bf6b..d8c4bf57e 100644 --- a/tests/unit/test__gapic.py +++ b/tests/unit/test__gapic.py @@ -114,14 +114,16 @@ def test_list_logs_with_max_results(self): with mock.patch.object( type(client._gapic_api.transport.list_log_entries), "__call__" ) as call: - call.return_value = logging_v2.types.ListLogEntriesResponse(entries=[log_entry_msg, log_entry_msg]) + call.return_value = logging_v2.types.ListLogEntriesResponse( + entries=[log_entry_msg, log_entry_msg] + ) result = client.list_entries( [PROJECT_PATH], filter_=FILTER, order_by=google.cloud.logging.ASCENDING, page_size=42, page_token="token", - max_results=1 + max_results=1, ) # Check the request @@ -142,7 +144,7 @@ def test_list_logs_negative_max_results(self): order_by=google.cloud.logging.ASCENDING, page_size=42, page_token="token", - max_results=-1 + max_results=-1, ) # Check the request list(result) @@ -258,7 +260,9 @@ def test_list_sinks_with_max_results(self): with mock.patch.object( type(client._gapic_api.transport.list_sinks), "__call__" ) as call: - call.return_value = logging_v2.types.ListSinksResponse(sinks=[sink_msg, sink_msg]) + call.return_value = logging_v2.types.ListSinksResponse( + sinks=[sink_msg, sink_msg] + ) result = client.list_sinks( self.PARENT_PATH, page_size=42, page_token="token", max_results=1 ) @@ -460,7 +464,9 @@ def test_list_metrics_with_max_results(self): with mock.patch.object( type(client._gapic_api.transport.list_log_metrics), "__call__" ) as call: - call.return_value = logging_v2.types.ListLogMetricsResponse(metrics=[metric, metric]) + call.return_value = logging_v2.types.ListLogMetricsResponse( + metrics=[metric, metric] + ) result = client.list_metrics( PROJECT, page_size=42, page_token="token", max_results=1 )