Add Error Reporting GAPIC as dependency by waprin · Pull Request #2894 · googleapis/google-cloud-python

waprin · 2016-12-21T02:49:17Z

Step 1 to resolving #2687. See that discussion thread for the other followups.

daspecster · 2016-12-21T03:01:41Z

@waprin I forgot, can you assign people and add labels to PRs?

waprin · 2016-12-21T03:03:29Z

I can't. Though my teammates could change that for me, up to you guys if you want us to.

bjwatson · 2016-12-21T12:55:45Z

@waprin I gave you one-off write access to this repo. We can refactor this as a Github team if there are other Stackdriver devs we should include (especially if you need access to multiple google-cloud-* repos).

FYI @jgeewax @daspecster @dhermes @tseaver @omaray @danoscarmike

bjwatson · 2016-12-21T12:56:52Z

LGTM, but I'll let @tseaver or @dhermes sign off.

dhermes · 2016-12-21T15:21:40Z

This PR is pointless unless the package actually uses the depended upon package?

waprin · 2016-12-21T20:08:08Z

@dhermes the next incoming PR will actually use this client, but it's still not pointless, the idea is that we keep the pip install experience consistent whether it's handwritten or a GAPIC. You install google-cloud-* to get the clients, regardless of if the are hand-written, GAPICs, or a mix. So there will be a bunch of APIs that just have a GAPIC we just install the dependency here for. Or in cases like this, a GAPIC and then maybe some hand-written code on top. At least that's how it's been explained to me.

dhermes · 2016-12-21T20:22:08Z

Yes, that's how it's been explained to me as well, but AFAIK the generated libraries are not in the "ready to go" state yet (e.g. no docs).

bjwatson · 2016-12-22T16:19:47Z

@dhermes We'll get on top of publishing GAPIC docs for all APIs in January. We also need to discuss how to integrate them with google-cloud-python docs as discussed in https://github.com/GoogleCloudPlatform/gcloud-common/issues/207.

waprin · 2016-12-29T20:41:09Z

@dhermes @bjwatson @steren this is not remotely ready for thorough review (missing tests, docs, lots of stye issues) but I wanted to throw up the commits to give the rough sketch of adding the GAPIC as a transport for the helper API, make sure it's all LGTM to you guys and then I'll clean up the PR to merge it.

This is basically what Steren and I had discussed a few weeks ago.

Copies the basic GAX/non-GAX structure of the logging client
Unlike the logging client, instead of falling back to an HTTP client, it falls back to the logging client itself, now in _logging
Basic instrumentation/helper library API remains the same, code to build the error report payload is shared

If you all feel good about this I'll finish up the tests/docs/style issues.

Obviously not expecting any response on this until 2017, happy new year! 🎉 🎉 🎈 🎈

waprin · 2017-01-03T21:00:45Z

@dhermes @bjwatson @steren ping for the new year. again, just looking for an initial +1 to basic direction here before I finish tests/docs.

bjwatson · 2017-01-04T01:37:49Z

Thanks @waprin. Looks reasonable to me based on a superficial review.

@lukesneeringer Can you take a more detailed look? This should have a similar structure to the Logging code that you've looked at already.

waprin · 2017-01-04T02:15:59Z

Cool ty, will finish it up then.

steren · 2017-01-04T09:28:39Z

The direction LGTM.

Would you mind also update the doc, so that I better understand how the end user will configure and use the module in its various variants?

In general, a parameter like use_gax is very cryptic to anybody who does not know what gax is (I guess most users).

Will this work also allow to call, for example, the list groupStat endpoint from the module?

waprin · 2017-01-12T01:45:01Z

@bjwatson @lukesneeringer cool ty, I wanted to ask, do we need the secure channel stuff? Is there a high level overview of that?

One other problem I've run into is that Error Reporting depends on gax 0.14 but logging (which Error Reporting uses as the fallback) depends on 0.15, and ssl_creds changed to ssl_credentials. Wonder if we can just update the error reporting gax dependency?

@steren will try to clarify the gRPC stuff.

Since we are adding the GAPIC to the module, all the autogenned VTK calls will be included as a dependency. Documentation of all those calls is still a TODO as noted above. For now this just adds the GAPIC library to this one and uses it for the handwritten helper functions.

bjwatson · 2017-01-12T01:49:25Z

@waprin I believe we'll need to regenerate the GAPIC code to work with GAX 0.15. Is that correct @geigerj?

The changes allow us to use @jonparrott's new google-auth library, rather than requiring the broken old oauth2client.

geigerj · 2017-01-12T05:34:56Z

@bjwatson Yes, we should regenerate the GAPIC library to use GAX 0.15.0.

waprin · 2017-01-30T17:42:01Z

@dhermes this is ready for review

lukesneeringer

Mostly seems fine -- there are a couple spots where I need clarification (or possibly changes).

Sign in to view

 import traceback

-import google.cloud.logging.client
+try:


Sign in to view

+
+from google.cloud.gapic.errorreporting.v1beta1 import (
+ report_errors_service_client)
+from google.cloud.grpc.devtools.clouderrorreporting.v1beta1 import (


* Add Error Reporting GAPIC as dependency This both adds the GAPIC to be used directly via this package, as well as hooking it up to the existing helpers to provide gRPC support.

…udPlatform/python-docs-samples#3698) fixes#2894 Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com>

…form/python-docs-samples#3765) fixes#2894

* Add Error Reporting GAPIC as dependency This both adds the GAPIC to be used directly via this package, as well as hooking it up to the existing helpers to provide gRPC support.

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

waprin mentioned this pull request Dec 21, 2016
Add Error Reporting gRPC #2687
Closed

daspecster assigned dhermes Dec 21, 2016

daspecster added the error reporting label Dec 21, 2016

daspecster assigned tseaver Dec 21, 2016

waprin force-pushed the gapic_errors branch from 739246a to 2ee9e38Compare December 29, 2016 20:35

waprin changed the title ~~Add Error Reporting GAPIC as dependency~~Add GAX/gRPC Transport to Error ReportingDec 29, 2016

waprin changed the title ~~Add GAX/gRPC Transport to Error Reporting~~[ROUGH DRAFT] Add Error Reporting GAPIC as dependencyDec 29, 2016

waprin force-pushed the gapic_errors branch from 2ee9e38 to c3e0235Compare January 27, 2017 21:08

waprin changed the title ~~[ROUGH DRAFT] Add Error Reporting GAPIC as dependency~~Add Error Reporting GAPIC as dependencyJan 27, 2017

lukesneeringer suggested changes Jan 31, 2017
View reviewed changes

waprin force-pushed the gapic_errors branch from 2377143 to 2292519Compare February 9, 2017 23:59

lukesneeringer approved these changes Feb 11, 2017
View reviewed changes

Bill Prin added 14 commits February 14, 2017 13:27

Add Error Reporting GAPIC as dependency
28f4c98

Add GAX to Error Reporting
df26d80

fixing up stuff
e2eb576

Add tests, make Travis pass
e92e17d

Add tests
128cc3a

Minor fixup
a51d687

Fix coverage
e6c3547

More mixups
de8daaa

Fix coverage
4b9c8fa

Add credentials
602e313

Fix determine
88873ba

Fix up
3d956a5

sneeringer review
fa382b3

typos
58b1e76

waprin force-pushed the gapic_errors branch from 2292519 to 58b1e76Compare February 14, 2017 21:27

waprin merged commit ee4f2ad into googleapis:masterFeb 14, 2017

dhermes added api: clouderrorreporting Issues related to the Error Reporting API. and removed api: clouderrorreporting Issues related to the Error Reporting API. labels Feb 23, 2017

dhermes mentioned this pull request Feb 24, 2017
Upgrading all versions for umbrella release. #3068
Merged
19 tasks

theacodes unassigned dhermes Sep 28, 2018

parthea pushed a commit that referenced this pull request Oct 21, 2023
[container_registry] fix: bump the pubsub timeout [(#3698)](GoogleClo…
2e7c129
…udPlatform/python-docs-samples#3698) fixes#2894 Co-authored-by: Leah E. Cole <6719667+leahecole@users.noreply.github.com>

parthea pushed a commit that referenced this pull request Oct 21, 2023
[container-registry] fix: mark a flaky test [(#3765)](GoogleCloudPlat…
2a2a7b2
…form/python-docs-samples#3765) fixes#2894

Uh oh!

Conversation

waprin commented Dec 21, 2016

Uh oh!

daspecster commented Dec 21, 2016

Uh oh!

waprin commented Dec 21, 2016

Uh oh!

bjwatson commented Dec 21, 2016

Uh oh!

bjwatson commented Dec 21, 2016

Uh oh!

dhermes commented Dec 21, 2016

Uh oh!

waprin commented Dec 21, 2016

Uh oh!

dhermes commented Dec 21, 2016

Uh oh!

bjwatson commented Dec 22, 2016

Uh oh!

waprin commented Dec 29, 2016

Uh oh!

waprin commented Jan 3, 2017

Uh oh!

bjwatson commented Jan 4, 2017

Uh oh!

waprin commented Jan 4, 2017

Uh oh!

steren commented Jan 4, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

waprin commented Jan 12, 2017

Uh oh!

bjwatson commented Jan 12, 2017

Uh oh!

geigerj commented Jan 12, 2017

Uh oh!

waprin commented Jan 30, 2017

Uh oh!

lukesneeringer left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

steren commented Jan 4, 2017•
edited
Loading