Skip to content

Regression of support for log level constants #104

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
zroger opened this issue Aug 14, 2020 · 8 comments
Closed

Regression of support for log level constants #104

zroger opened this issue Aug 14, 2020 · 8 comments
Assignees
Labels
bug Something isn't working need-more-information Pending information to continue

Comments

@zroger
Copy link

zroger commented Aug 14, 2020

Before #99 it was possible to pass a logging constant like logging.INFO to the Logger constructor. After #99, I get this exception: AttributeError: 'int' object has no attribute 'upper' from logging/logger.py:122.

The docstrings are also confusing, since the environment variable says it accepts and integer or a string, but the constructor parameter is a string.

Expected Behavior

  • The Logger constructor should accept logging constants (integers) as well as strings.
  • The environment variable and constructor arg for log level should be consistent
  • The docstrings should be consistent with the code

Current Behavior

When an integer is passed, there is an AttributeError.

Steps to Reproduce (for bugs)

>>> import logging
>>> from aws_lambda_powertools import Logger
>>> Logger(service="demo", level=logging.INFO)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.8/site-packages/aws_lambda_powertools/logging/logger.py", line 122, in __init__
    self.log_level = self._get_log_level(level)
  File "/usr/local/lib/python3.8/site-packages/aws_lambda_powertools/logging/logger.py", line 138, in _get_log_level
    log_level = log_level.upper() if log_level is not None else logging.INFO
AttributeError: 'int' object has no attribute 'upper'

Environment

  • Powertools version used: 1.1.0
  • Packaging format (Layers, PyPi):
  • AWS Lambda function runtime: python3.8
  • Debugging logs
@zroger zroger added bug Something isn't working triage Pending triage from maintainers labels Aug 14, 2020
@heitorlessa
Copy link
Contributor

Looking into this now...

@heitorlessa
Copy link
Contributor

Just managed to reproduce in a quick test - Working on a fix and should be upstream shortly. Thanks for raising this quickly @zroger

@heitorlessa
Copy link
Contributor

PR: #105

Updating Changelog, and reviewing your comment on docstring before I publish it

@heitorlessa
Copy link
Contributor

heitorlessa commented Aug 14, 2020

What it's quite odd is monkeypatch when creating a test for env var it always casts the value as string despite setting as int -- Can't reproduce in zsh/bash tho

def test_logger_level_env_var_as_int(monkeypatch):
    # GIVEN Logger is initialized
    # WHEN log level is set as int defined via LOG_LEVEL env
    monkeypatch.setenv("LOG_LEVEL", 50)
    logger = Logger()

    # THEN log level should be equals to 50 => CRITICAL
    assert logger.level == logging.CRITICAL

I'll continue on digging.

UPDATE: It's an issue with test runner which converts all int value in env var to str. I'll send a PR to cater for integer env var w/o a test, and will push a test later as it won't impact the release either way.

UPDATE 2: Take this back. Environment variables cannot be integer but string hence Pytest runner implicitly converting it and emitting a warning. Just created a fresh Sam build, and it's always string. Created a fresh docker container to verify under python image while overriding entrypoint and it's the same behaviour. Re-adding the LOG_LEVEL env var docstring to str only.

@zroger
Copy link
Author

zroger commented Aug 14, 2020

Thanks for handling this so quickly

@heitorlessa
Copy link
Contributor

Published to PyPi but seems to be taking longer than the usual to reflect 1.1.1 - Will circle back here once it's through

@heitorlessa
Copy link
Contributor

heitorlessa commented Aug 14, 2020

Damn page cache haha just managed to install by passing the latest version directly: pip install aws-lambda-powertools==1.1.1

Lemme know if that works for you and we can close this -- Thank you @zroger again for reporting it

image

@heitorlessa heitorlessa added need-more-information Pending information to continue and removed triage Pending triage from maintainers labels Aug 15, 2020
@heitorlessa
Copy link
Contributor

Alright, I'm closing this now as tests are covering this now, and have just released 1.1.2 to address PyCharm/VSCode Jedi Language Server autocomplete issue -- PyPi is operating as expected now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working need-more-information Pending information to continue
Projects
Development

No branches or pull requests

2 participants