Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore!: deprecate AppEngineHandler and ContainerEngineHandler #298

Closed
wants to merge 21 commits into from

Conversation

daniel-sanche
Copy link
Contributor

This is the first PR for the upcoming 3.0.0 release. Removes AppEngineHandler and ContainerEngineHandler in favor of the more generic CloudLoggingHandler (for network logging) and StructuredLogHandler (for standard output JSON logging).

Fixes #202

@daniel-sanche daniel-sanche requested review from a team as code owners May 13, 2021 21:12
@product-auto-label product-auto-label bot added the api: logging Issues related to the googleapis/python-logging API. label May 13, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label May 13, 2021
@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 13, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 13, 2021
@daniel-sanche daniel-sanche changed the title [DRAFT] chore: deprecate AppEngineHandler and ContainerEngineHandler chore: deprecate AppEngineHandler and ContainerEngineHandler May 18, 2021
@daniel-sanche daniel-sanche changed the title chore: deprecate AppEngineHandler and ContainerEngineHandler chore!: deprecate AppEngineHandler and ContainerEngineHandler May 18, 2021
Comment on lines -332 to -334
:class:`~google.cloud.logging.handlers.app_engine.AppEngineHandler`, which is
recommended when running on the Google App Engine Flexible vanilla runtimes
(i.e. your app.yaml contains ``runtime: python``), and

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should mark the classes as deprecated: update the docs to mention that they are not returned anymore and that new classes replace them. Folks who have existing code using 2.* library release should have a path forward that explains how to upgrade to 3.0.0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My plan was to go through and update the docs at the end of the milestone, just in case any docs changes become out of date later (#245)

Do you think that makes sense, or should I try to update the docs along-side changes?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can update the docs later. The point here is more about the annotation of the classes.

@@ -352,9 +350,9 @@ def get_default_handler(self, **kw):

if isinstance(monitored_resource, Resource):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daniel-sanche is it possible to document the behavior of get_default_handler more thoroughly somewhere, including the default for each environment? I think some users will be initially confused why their logs are showing up in stdout and/or whichever other sinks they might be streaming their stdout logs to

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this suggestion to the open issue around updating the docs for v3.

Let me know if you have thoughts about whether docs should be updated along with each change, or as a pass at the end of the milestone

@daniel-sanche daniel-sanche mentioned this pull request May 19, 2021
2 tasks
@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 27, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 27, 2021
@daniel-sanche
Copy link
Contributor Author

closed in favor of #310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the googleapis/python-logging API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants