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

[UX] Fix unnecessary OCI logging #4476

Merged
merged 1 commit into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions sky/adaptors/oci.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
"""Oracle OCI cloud adaptor"""

import logging
import os

from sky.adaptors import common

# Suppress OCI circuit breaker logging before lazy import, because
# oci modules prints additional message during imports, i.e., the
# set_logger in the LazyImport called after imports will not take
# effect.
logging.getLogger('oci.circuit_breaker').setLevel(logging.WARNING)

CONFIG_PATH = '~/.oci/config'
ENV_VAR_OCI_CONFIG = 'OCI_CONFIG'

Expand Down
65 changes: 65 additions & 0 deletions tests/unit_tests/sky/adaptors/test_oci.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
"""Tests for OCI adaptor."""
import logging

import pytest

from sky import check as sky_check
from sky.adaptors import oci
from sky.utils import log_utils


def test_oci_circuit_breaker_logging():
"""Test that OCI circuit breaker logging is properly configured."""
# Get the circuit breaker logger
logger = logging.getLogger('oci.circuit_breaker')

# Create a handler that captures log records
log_records = []
test_handler = logging.Handler()
test_handler.emit = lambda record: log_records.append(record)
logger.addHandler(test_handler)

# Create a null handler to suppress logs during import
null_handler = logging.NullHandler()
logger.addHandler(null_handler)

try:
# Verify logger starts at WARNING level (set by adaptor initialization)
initial_level = logger.getEffectiveLevel()
print(
f'Initial logger level: {initial_level} (WARNING={logging.WARNING})'
)
assert initial_level == logging.WARNING, (
'OCI circuit breaker logger should be set to WARNING before initialization'
)

# Force OCI module import through LazyImport by accessing a module attribute
print('Attempting to import OCI module...')
try:
# This will trigger LazyImport's load_module for the actual OCI module
_ = oci.oci.config.DEFAULT_LOCATION
except (ImportError, AttributeError) as e:
# Expected when OCI SDK is not installed
print(f'Import/Attribute error as expected: {e}')
pass

# Verify logger level after import attempt
after_level = logger.getEffectiveLevel()
print(
f'Logger level after import: {after_level} (WARNING={logging.WARNING})'
)
assert after_level == logging.WARNING, (
'OCI circuit breaker logger should remain at WARNING after initialization'
)

# Verify no circuit breaker logs were emitted
circuit_breaker_logs = [
record for record in log_records
if 'Circuit breaker' in record.getMessage()
]
assert not circuit_breaker_logs, (
'No circuit breaker logs should be emitted during initialization')
finally:
# Clean up the handlers
logger.removeHandler(test_handler)
logger.removeHandler(null_handler)
Loading