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

[tseaver]

Uses #1574 as a base.

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

[tseaver]

@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.

[dhermes]

@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)

[dhermes]

I take it this hasn't been rebased yet?

[tseaver]

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.

[tseaver]

I take it this hasn't been rebased yet?

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

[dhermes]

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

[tseaver]

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.

[dhermes]

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

[tseaver]

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'.

[dhermes]

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

[tseaver]

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

[dhermes]

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.

[tseaver]

@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

[dhermes]

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?

[tseaver]

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"
  },
...
 ]
}

[dhermes]

Really helpful! Thanks

[dhermes]

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

[tseaver]

@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.

[dhermes]

SGTM thanks