Skip to content

Importing the sagemaker module change the logging setup #755

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

Closed
eprochasson opened this issue Apr 18, 2019 · 3 comments
Closed

Importing the sagemaker module change the logging setup #755

eprochasson opened this issue Apr 18, 2019 · 3 comments
Labels
status: pending release The fix have been merged but not yet released to PyPI type: bug

Comments

@eprochasson
Copy link

eprochasson commented Apr 18, 2019

System Information

  • Framework (e.g. TensorFlow) / Algorithm (e.g. KMeans): NA
  • Framework Version: NA
  • Python Version: 3.6.3
  • CPU or GPU: NA
  • Python SDK Version: sagemaker-1.18.14.post1 (current version served by pip install sagemaker)
  • Are you using a custom image: no

Describe the problem

Simply importing the sagemaker module alterates the logging module configuration.

Minimal repro / logs

Consider the following code:

import logging

logging.basicConfig(level='DEBUG',
                    format='%(asctime)s %(name)-12s %(levelname)-8s %(message)s',
                    datefmt='%m-%d %H:%M')

logging.debug("debug")
logging.info('info')
logging.error('error')
logging.warning("WARNING")

When ran, will output:

04-18 09:15 root         DEBUG    debug
04-18 09:15 root         INFO     info
04-18 09:15 root         ERROR    error
04-18 09:15 root         WARNING  WARNING

as expected. Now, the modified code, the only difference being the import of the sagemaker module:

import logging
import sagemaker

logging.basicConfig(level='DEBUG',
                    format='%(asctime)s %(name)-12s %(levelname)-8s %(message)s',
                    datefmt='%m-%d %H:%M')

logging.debug("debug")
logging.info('info')
logging.error('error')
logging.warning("WARNING")

will output:

WARNING:root:pandas failed to import. Analytics features will be impaired or broken.
ERROR:root:error
WARNING:root:WARNING

The logging level and formatting has been overwritten (the warning about pandas is not the problem I'm pointing at).

@eprochasson
Copy link
Author

eprochasson commented Apr 18, 2019

I think the problem arise because:

  • I import sagemaker before first calling logging.basicConfig
  • in multiple places in the sagemaker code, I see:
logging.basicConfig()
LOGGER = logging.getLogger('sagemaker')
LOGGER.setLevel(logging.INFO)

which does call logging.basicConfig(). According to the python documentation about logging:

This function does nothing if the root logger already has handlers configured for it.

I don't think a module should configure any logging parameters (i.e.: only the line LOGGER = logging.getLogger('sagemaker') should remain).

Workaround: do not import sagemaker before calling logging.basicConfig() at a higher level.

@laurenyu
Copy link
Contributor

hi @eprochasson, thanks for the detailed bug report! I've created a PR to address this bug: #757

@laurenyu laurenyu added the status: pending release The fix have been merged but not yet released to PyPI label Apr 19, 2019
@laurenyu
Copy link
Contributor

the changes have now been released. closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: pending release The fix have been merged but not yet released to PyPI type: bug
Projects
None yet
Development

No branches or pull requests

2 participants