Skip to content

Conversation

@trptcolin
Copy link
Contributor

For example: SENTRY_LOG_LEVEL=ERROR, etc.

Per discussion on #21

I did preserve the existing behavior (e.g. SENTRY_LOG_LEVEL=40) to avoid breaking API changes, and I also added some tests around most of the current defaults.

@coveralls
Copy link

coveralls commented Mar 16, 2018

Coverage Status

Coverage increased (+2.5%) to 89.272% when pulling 6a6a812 on trptcolin:log-level-via-string into 2a290a6 on Netflix-Skunkworks:master.

'is_local': os.environ.get('IS_OFFLINE', False) or os.environ.get('IS_LOCAL', False),
'logging': boolval(os.environ.get('SENTRY_CAPTURE_LOGS', True)),
'log_level': int(os.environ.get('SENTRY_LOG_LEVEL', logging.WARNING)),
'log_level': extract_log_level_from_environment('SENTRY_LOG_LEVEL', logging.WARNING),
Copy link

Choose a reason for hiding this comment

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

Simpler way which doesnt require a new function:

log_levels.get(os.environ.get('SENTRY_LOG_LEVEL'), logging.WARNING)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would work for the new use case but doesn't preserve backwards compatibility (setting SENTRY_LOG_LEVEL to a numeric value).

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed -- LGTM.

@mikegrima
Copy link
Contributor

@trptcolin : Great PR 👍

Can you bump up the version number in: https://github.com/Netflix-Skunkworks/raven-python-lambda/blob/master/raven_python_lambda/__about__.py#L12, and I will then merge and push the new release to PyPI.

@mikegrima mikegrima merged commit f06e664 into Netflix-Skunkworks:master Mar 16, 2018
@mikegrima
Copy link
Contributor

0.1.7 uploaded to PyPI.

@trptcolin trptcolin deleted the log-level-via-string branch March 16, 2018 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants