Skip to content

Commit

Permalink
Fix: Give aiplatform logging its own log namespace, let the user conf…
Browse files Browse the repository at this point in the history
…igure their own root logger (#1081)

* Don't throw exception when getting representation of GCA resources not yet created

* Fixes for PR feedback

* Linted changes

* Fixes for PR feedback

* Linted changes

* Base.py no longer sets root logger config

* Added logging tests

* Removed dangerous pattern encouraging setting of custom logger namespace names

* Blacken formatting changes

* Added python 3.6 compatability for setting stream handler

* Removed unused imports

* Revert "Removed dangerous pattern encouraging setting of custom logger namespace names"

This reverts commit 35fdda8.

* Removed optional logger name param

* Added explicit __name__ as param for test

* Rerun tests

Co-authored-by: Sam Goodman <[email protected]>
Co-authored-by: Karl Weinmeister <[email protected]>
  • Loading branch information
3 people authored Mar 30, 2022
1 parent b5f42bd commit fb78243
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 4 deletions.
12 changes: 8 additions & 4 deletions google/cloud/aiplatform/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,26 @@
from google.cloud.aiplatform.compat.types import encryption_spec as gca_encryption_spec
from google.protobuf import json_format

logging.basicConfig(level=logging.INFO, stream=sys.stdout)

# This is the default retry callback to be used with get methods.
_DEFAULT_RETRY = retry.Retry()


class Logger:
"""Logging wrapper class with high level helper methods."""

def __init__(self, name: str = ""):
"""Initializes logger with name.
def __init__(self, name: str):
"""Initializes logger with optional name.
Args:
name (str): Name to associate with logger.
"""
self._logger = logging.getLogger(name)
self._logger.setLevel(logging.INFO)

handler = logging.StreamHandler(sys.stdout)
handler.setLevel(logging.INFO)

self._logger.addHandler(handler)

def log_create_with_lro(
self,
Expand Down
51 changes: 51 additions & 0 deletions tests/unit/aiplatform/test_logging.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# -*- coding: utf-8 -*-

# Copyright 2020 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

from google.cloud.aiplatform import base
import logging


class TestLogging:
def test_no_root_logging_handler_override(self, caplog):
# Users should be able to control the root logger in their apps
# The aiplatform module import should not override their root logger config
caplog.set_level(logging.DEBUG)

logging.debug("Debug level")
logging.info("Info level")
logging.critical("Critical level")

assert "Debug level\n" in caplog.text
assert "Info level\n" in caplog.text
assert "Critical level\n" in caplog.text

def test_log_level_coexistance(self, caplog):
# The aiplatform module and the root logger can have different log levels.
aip_logger = base.Logger(__name__)

caplog.set_level(logging.DEBUG)

logging.debug("This should exist")
logging.info("This should too")

aip_logger.info("This should also exist")
aip_logger.debug("This should NOT exist")

assert "This should exist\n" in caplog.text
assert "This should too\n" in caplog.text
assert "This should also exist\n" in caplog.text
assert "This should NOT exist\n" not in caplog.text

0 comments on commit fb78243

Please sign in to comment.