Add support for parsing log entries w/ 'protoPayload' from resources by tseaver · Pull Request #1578 · googleapis/google-cloud-python · GitHub

Uses #1574 as a base.

Note that we can't feasibly allow writing such entries until we have an answer for #1577.

@dhermes 8d5ae2b is the commit which disables the too-many-lines/too-many-statements limits altogether for test code. That feels cleaner to me than either disabling the limit file-by-file, or repeatedly nudging it upwards.

@tseaver too-many-lines / too-many-statements should not be disabled. They provide valuable feedback when we've maybe gone a little too far. e.g. when the row.py cartel should've been broken up (#1602)

I take it this hasn't been rebased yet?

too-many-lines / too-many-statements

For testing code, I disagree: "it takes as much as it takes" to cover the MUT, and the arbitrary restriction on testcase method length tends to lead to "getting fancy" in testing code.

I take it this hasn't been rebased yet?

It is current with the tip of the logging-api branch.

RE: Rebased, the "Add 'Logger.list_entries'." commit must not have merged cleanly then. Maybe you want to cherry-pick the last two commits?

Github seems confused: e4b4b0d is already present on the logging-api branch, When I try to rebase against logging-api in my local checkout, that commit does not appear anywhere.

Try a bit more to unconfuse it? Maybe re-fetch the remote locally?

How? The histories look identical to me:

$ git pull upstream logging-api 
From github.com:GoogleCloudPlatform/gcloud-python
 * branch            logging-api -> FETCH_HEAD
Already up-to-date.

$ git log -3 upstream/logging-api
commit b8151b1e0f7dbf7b9b5a5beff65ca49042798a99
Merge: 68baa1b e4b4b0d
Author: Tres Seaver <tseaver@palladion.com>
Date:   Fri Mar 11 12:44:00 2016 -0500

    Merge pull request #1574 from tseaver/logging-logger_list_entries

    Add 'Logger.list_entries'.

commit e4b4b0d2d1a6bfb9348ff2f073a9577cdf03c378
Author: Tres Seaver <tseaver@palladion.com>
Date:   Wed Mar 9 16:17:14 2016 -0500

    Add 'Logger.list_entries'.

commit 68baa1b05ac0b15e608397756790bb5f06d14846
Merge: e898bca d7a2e72
Author: Tres Seaver <tseaver@palladion.com>
Date:   Fri Mar 11 11:55:02 2016 -0500

    Merge pull request #1569 from tseaver/logging-client_list_entries

    Add 'Client.list_entries'.

$ git log -3 logging-api
commit b8151b1e0f7dbf7b9b5a5beff65ca49042798a99
Merge: 68baa1b e4b4b0d
Author: Tres Seaver <tseaver@palladion.com>
Date:   Fri Mar 11 12:44:00 2016 -0500

    Merge pull request #1574 from tseaver/logging-logger_list_entries

    Add 'Logger.list_entries'.

commit e4b4b0d2d1a6bfb9348ff2f073a9577cdf03c378
Author: Tres Seaver <tseaver@palladion.com>
Date:   Wed Mar 9 16:17:14 2016 -0500

    Add 'Logger.list_entries'.

commit 68baa1b05ac0b15e608397756790bb5f06d14846
Merge: e898bca d7a2e72
Author: Tres Seaver <tseaver@palladion.com>
Date:   Fri Mar 11 11:55:02 2016 -0500

    Merge pull request #1569 from tseaver/logging-client_list_entries

    Add 'Client.list_entries'.

$ git log -5 logging-parse_protobuf_entries 
commit 124ccf08c929794f2cd4ba5ba9fef3d9fdbf42d3
Author: Tres Seaver <tseaver@palladion.com>
Date:   Mon Mar 7 11:37:16 2016 -0500

    Add support for parsing log entries w/ 'protoPayload' from resources.

    We can't feasibly allow writing such entries until we have an answer for

commit 465699c255e39132cc075e114f44807150d9cce2
Author: Tres Seaver <tseaver@palladion.com>
Date:   Mon Mar 7 11:35:57 2016 -0500

    Disable 'too-many-statements'/'too-many-lines' for test code.

commit b8151b1e0f7dbf7b9b5a5beff65ca49042798a99
Merge: 68baa1b e4b4b0d
Author: Tres Seaver <tseaver@palladion.com>
Date:   Fri Mar 11 12:44:00 2016 -0500

    Merge pull request #1574 from tseaver/logging-logger_list_entries

    Add 'Logger.list_entries'.

commit e4b4b0d2d1a6bfb9348ff2f073a9577cdf03c378
Author: Tres Seaver <tseaver@palladion.com>
Date:   Wed Mar 9 16:17:14 2016 -0500

    Add 'Logger.list_entries'.

commit 68baa1b05ac0b15e608397756790bb5f06d14846
Merge: e898bca d7a2e72
Author: Tres Seaver <tseaver@palladion.com>
Date:   Fri Mar 11 11:55:02 2016 -0500

    Merge pull request #1569 from tseaver/logging-client_list_entries

    Add 'Client.list_entries'.

Looks like you've sorted it out. Only two commits now in this PR.

I just tried a new push -f from the loggin-parse_protobuf_entries branch, which seems to have cleared the stray commit.

For testing code, I disagree: "it takes as much as it takes" to cover the MUT, and the arbitrary restriction on testcase method length tends to lead to "getting fancy" in testing code.

It's not hard to argue that code like the lint-disabled test_upload_from_file_w_explicit_client_resumable is hard to follow. These restrictions give us a signal which is valuable to us when we get too deep in our own code and lose context.

We can't feasibly allow writing such entries until we have an answer for

@dhermes 7c52009 drops suppressing the project-wide 'too-many-statements' check for test code, and instead disables it locally for the one newly-added test: looking at it, I don't see any reason to break it up. PTAL

Only real pending issue is my comment about seeing an example of a proto payload from a real API request. Have you been able to produce one?

From the "Try It" form:

POST https://logging.googleapis.com/v2beta1/entries:list?key={YOUR_API_KEY}

{
 "projectIds": [
  "citric-celerity-697"
 ]
}

returns:

200 OK

- Hide headers -

Cache-Control:  private
Content-Encoding:  gzip
Content-Length:  5768
Content-Type:  application/json; charset=UTF-8
Date:  Tue, 22 Mar 2016 01:52:27 GMT
Server:  ESF
Vary:  Origin, X-Origin, Referer

{
 "entries": [
  {
   "textPayload": "Logging from gcloud-python!",
   "insertId": "2016-03-03|11:28:12.764719-08|10.95.84.10|1527583067",
   "resource": {
    "type": "global"
   },
   "timestamp": "2016-03-03T19:28:12.750563482Z",
   "logName": "projects/citric-celerity-697/logs/hacking-around"
  },
  {
   "insertId": "2016-03-03|11:48:07.740612-08|10.95.212.81|-486515530",
   "jsonPayload": {
    "weather": "cloudy",
    "message": "Now with arbitrary key-values!"
   },
   "resource": {
    "type": "global"
   },
   "timestamp": "2016-03-03T19:48:07.728837588Z",
   "logName": "projects/citric-celerity-697/logs/hacking-around"
  },
  {
   "protoPayload": {
    "@type": "type.googleapis.com/google.cloud.audit.AuditLog",
    "status": {
    },
    "authenticationInfo": {
    },
    "requestMetadata": {
    },
    "serviceName": "bigquery.googleapis.com",
    "methodName": "datasetservice.list",
    "authorizationInfo": [
     {
      "resource": "projects/citric-celerity-697",
      "permission": "bigquery.datasets.list",
      "granted": true
     },
     {
      "resource": "projects/citric-celerity-697/datasets/test_listing_datasets",
      "permission": "bigquery.datasets.get",
      "granted": true
     }
    ],
    "resourceName": "projects/citric-celerity-697/datasets",
    "serviceData": {
     "@type": "type.googleapis.com/google.cloud.bigquery.logging.v1.AuditData",
     "datasetListRequest": {
     }
    }
   },
   "insertId": "52D3EC4A89C80.A5FD1C7.7C0038C9",
   "resource": {
    "type": "bigquery_resource"
   },
   "timestamp": "2016-03-04T20:14:54.160Z",
   "severity": "INFO",
   "logName": "projects/citric-celerity-697/logs/cloudaudit.googleapis.com%2Fdata_access"
  },
...
 ]
}

Really helpful! Thanks

LGTM (though I still think reducing the test boilerplate for the do-nothing subclasses would be a net positive)

Addresses: #1578 (comment)

@dhermes I changed gcloud.logging.test_entries to test _BaseEntry directly, dropping tests for the method-less derived classes. I will merge when Travis passes.

SGTM thanks

Add support for parsing log entries w/ 'protoPayload' from resources

…#1578) * fix: use not implemented error when REST transport is not supported * lint * revert local changes * update error message and combine not implemented errors * fix typo * fix typo * update ads templates * formatting

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [x] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/python-pubsub/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) Fixes #1578 🦕

* add a new session and connect to test env * I need to revert code change * revert files for unncessary change * fix more tests * all test are clean * fix more tests * merge in the change * revert the order changes, move single-modality tests back to PROD since they are in GA * refactor multimodal tests out * change the project name * revert change * remove unused bq client

tseaver added the api: logging Issues related to the Cloud Logging API. label Mar 7, 2016

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 7, 2016

tseaver force-pushed the logging-api branch from 87bccf9 to d38fd23 Compare March 8, 2016 15:49

tseaver mentioned this pull request Mar 8, 2016

Where are the allowed log entry protobuf types documented? #1577

Closed

tseaver assigned dhermes Mar 11, 2016

dhermes reviewed Mar 11, 2016
View reviewed changes

Comment thread gcloud/logging/entries.py

See:

https://cloud.google.com/logging/docs/api/ref_v2beta1/rest/v2beta1/LogEntry

"""

_PAYLOAD_KEY = 'protoPayload'

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Add support for parsing log entries w/ 'protoPayload' from resources.

7c52009

We can't feasibly allow writing such entries until we have an answer for

Test '_BaseEntry' directly, drop tests for methodless subclasses.

fd8965c

Addresses: #1578 (comment)

tseaver added a commit that referenced this pull request Mar 22, 2016

Merge pull request #1578 from tseaver/logging-parse_protobuf_entries

9f37a05

Add support for parsing log entries w/ 'protoPayload' from resources

tseaver merged commit 9f37a05 into googleapis:logging-api Mar 22, 2016

tseaver deleted the logging-parse_protobuf_entries branch March 22, 2016 15:34

dhermes mentioned this pull request Apr 1, 2016

Upgrading version to 0.12.0 #1689

Merged

theacodes unassigned dhermes Sep 28, 2018

+                  See:
+                  https://cloud.google.com/logging/docs/api/ref_v2beta1/rest/v2beta1/LogEntry
+                  """
+                  _PAYLOAD_KEY = 'protoPayload'

Add support for parsing log entries w/ 'protoPayload' from resources by tseaver · Pull Request #1578 · googleapis/google-cloud-python · GitHub

Uh oh!

Add support for parsing log entries w/ 'protoPayload' from resources - #1578

Add support for parsing log entries w/ 'protoPayload' from resources#1578tseaver merged 2 commits intogoogleapis:logging-apigoogleapis/google-cloud-python:logging-apifrom tseaver:logging-parse_protobuf_entriesCopy head branch name to clipboard

Conversation

tseaver commented Mar 7, 2016

Uh oh!

tseaver commented Mar 11, 2016

Uh oh!

dhermes commented Mar 11, 2016

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

dhermes commented Mar 11, 2016

Uh oh!

tseaver commented Mar 11, 2016

Uh oh!

tseaver commented Mar 11, 2016

Uh oh!

dhermes commented Mar 11, 2016

Uh oh!

tseaver commented Mar 11, 2016

Uh oh!

dhermes commented Mar 11, 2016

Uh oh!

tseaver commented Mar 11, 2016

Uh oh!

dhermes commented Mar 11, 2016

Uh oh!

tseaver commented Mar 11, 2016

Uh oh!

dhermes commented Mar 11, 2016

Uh oh!

tseaver commented Mar 21, 2016

Uh oh!

dhermes commented Mar 22, 2016

Uh oh!

tseaver commented Mar 22, 2016

Uh oh!

dhermes commented Mar 22, 2016

Uh oh!

dhermes commented Mar 22, 2016

Uh oh!

tseaver commented Mar 22, 2016

Uh oh!

dhermes commented Mar 22, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add support for parsing log entries w/ 'protoPayload' from resources#1578
tseaver merged 2 commits into
googleapis:logging-apifrom
tseaver:logging-parse_protobuf_entries