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

consolidate logger logic to shared #818

Merged
merged 4 commits into from
Sep 19, 2018

Conversation

ryandeivert
Copy link
Contributor

to: @chunyong-lin
cc: @airbnb/streamalert-maintainers
size: medium

Background

I'm going to be adding another Lambda function soon, and it pains me to copy/pasta the old logger creation again.

Changes

I've removed how loggers were previously created and centralized in the shared module.

  • Adding central/shared logger.py along with tests
  • Updating all modules to use shared logger logic
  • Updating apps lambda packaging logic to include shared module

Testing

Updating all unit tests

@ryandeivert ryandeivert added this to the 2.0.0 milestone Sep 19, 2018
@ryandeivert ryandeivert force-pushed the ryandeivert-consolidate-logger branch from 7e115de to 25cfc32 Compare September 19, 2018 01:56
Copy link
Contributor

@chunyong-lin chunyong-lin left a comment

Choose a reason for hiding this comment

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

Beautiful!
LGTM, one question from my curiosity.

stream_alert/rule_processor/classifier.py Show resolved Hide resolved
@ryandeivert ryandeivert force-pushed the ryandeivert-consolidate-logger branch from 25cfc32 to 01e37c7 Compare September 19, 2018 17:42
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 97.31% when pulling 01e37c7 on ryandeivert-consolidate-logger into 777f423 on master.

@ryandeivert ryandeivert merged commit f05e2d4 into master Sep 19, 2018
@ryandeivert ryandeivert deleted the ryandeivert-consolidate-logger branch September 19, 2018 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants