Skip to content

Fix: Give aiplatform logging its own log namespace, let the user configure their own root logger #1081

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 25 commits into from
Mar 30, 2022
Merged

Fix: Give aiplatform logging its own log namespace, let the user configure their own root logger #1081

merged 25 commits into from
Mar 30, 2022

Conversation

samgoodman
Copy link
Contributor

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:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Addresses #967 and most of internal b/222347808
@samgoodman samgoodman changed the title Give aiplatform logging its own log namespace, let the user configure their own root logger Fix: Give aiplatform logging its own log namespace, let the user configure their own root logger Mar 15, 2022
This was referenced Mar 15, 2022
Closed
Closed
Ark-kun
Ark-kun reviewed Mar 15, 2022
View reviewed changes
@samgoodman samgoodman closed this Mar 29, 2022
@samgoodman samgoodman reopened this Mar 29, 2022
@samgoodman samgoodman requested a review from sasha-gitg March 30, 2022 16:05
sasha-gitg
sasha-gitg approved these changes Mar 30, 2022
View reviewed changes
Copy link
Member

@sasha-gitg sasha-gitg left a comment

Choose a reason for hiding this comment

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

LGTM with minor comments. Thanks Sam!
tests/unit/aiplatform/test_logging.py
@@ -0,0 +1,51 @@
# -*- coding: utf-8 -*-

# Copyright 2020 Google LLC
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2020 Google LLC
# Copyright 2022 Google LLC
google/cloud/aiplatform/base.py
def __init__(self, name: str = ""):
"""Initializes logger with name.
def __init__(self, name: str):
"""Initializes logger with optional name.
Copy link
Member

Choose a reason for hiding this comment

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

No longer optional.
@samgoodman samgoodman merged commit fb78243 into googleapis:main Mar 30, 2022
@release-please release-please bot mentioned this pull request Mar 30, 2022
Merged
@release-please release-please bot mentioned this pull request Jun 8, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants

@samgoodman @Ark-kun @morgandu @sasha-gitg @kweinmeister