Skip to content

Simplified logging#532

Merged
ejholmes merged 2 commits into
masterfrom
simple-logging
Feb 10, 2018
Merged

Simplified logging#532
ejholmes merged 2 commits into
masterfrom
simple-logging

Conversation

@ejholmes

@ejholmes ejholmes commented Feb 8, 2018

Copy link
Copy Markdown
Contributor

Closes #527

This removes the "loop logger", and replaces it with a simple sequential logger. I think this solves all of the problems talked about in #527:

  1. This will work with parallelized execution.
  2. You get color output with --interactive.
  3. It removes the superfluous "pending" status.
  4. It scales easily with large stack configs.

Here's an example of what the output looks like:

asciicast

@codecov-io

codecov-io commented Feb 8, 2018 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #532 into master will decrease coverage by 0.65%.
The diff coverage is 46.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #532      +/-   ##
==========================================
- Coverage    88.6%   87.94%   -0.66%     
==========================================
  Files          94       91       -3     
  Lines        5987     5875     -112     
==========================================
- Hits         5305     5167     -138     
- Misses        682      708      +26
Impacted Files Coverage Δ
stacker/commands/stacker/__init__.py 95.83% <ø> (ø) ⬆️
stacker/context.py 97.22% <ø> (-0.04%) ⬇️
stacker/logger/__init__.py 0% <0%> (-100%) ⬇️
stacker/actions/base.py 63.79% <100%> (-1.79%) ⬇️
stacker/plan.py 90.8% <100%> (-0.78%) ⬇️
stacker/commands/stacker/base.py 84.28% <75%> (-1.43%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 075b083...7dc3234. Read the comment docs.

@russellballestrini

russellballestrini commented Feb 8, 2018

Copy link
Copy Markdown
Member

Wow, this is refreshing, nice work!

@ejholmes ejholmes mentioned this pull request Feb 9, 2018
Merged
4 tasks
phobologic
phobologic approved these changes Feb 9, 2018
View reviewed changes
Comment thread stacker/logger/__init__.py
def setup_logging(verbosity):
log_level = logging.INFO
log_format = INFO_FORMAT
if sys.stdout.isatty():

@phobologic phobologic Feb 9, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

love this, thanks.

@phobologic

phobologic commented Feb 10, 2018

Copy link
Copy Markdown
Member

Ok, pushed a small update. @ejholmes can you take a look at why the test_util.py tests are all super noisy in tests now? See: https://circleci.com/gh/remind101/stacker/1404?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

(I thought it was something I had done, but it looks like it's been happening since this update)

@ejholmes ejholmes force-pushed the simple-logging branch 2 times, most recently from 6ee3ae4 to 1727ce0 Compare February 10, 2018 01:17
@ejholmes

ejholmes commented Feb 10, 2018

Copy link
Copy Markdown
Contributor Author

Tests were noisy because the root logger in tests was being switched out with a StreamLogger in stacker/tests/test_stacker.py.

I added a commit to this that dependency injects the setup_logging function from scripts/stacker so unit tests can't fuck with logging.

@ejholmes ejholmes merged commit b9242f6 into master Feb 10, 2018
@ejholmes ejholmes added this to the 1.2 milestone Mar 1, 2018
troyready added a commit to troyready/stacker that referenced this pull request Mar 22, 2018
@troyready
adapt cloudtools#532 to windows stacker.cmd
237e044
@troyready troyready mentioned this pull request Mar 22, 2018
Closed
ejholmes pushed a commit that referenced this pull request Mar 23, 2018
@troyready @ejholmes
adapt #532 to windows stacker.cmd
1f3b7c7
ejholmes pushed a commit that referenced this pull request Mar 23, 2018
@troyready @ejholmes
adapt #532 to windows stacker.cmd
042c6ec
aarongorka pushed a commit to amaysim-au/stacker that referenced this pull request May 4, 2018
@troyready
adapt cloudtools#532 to windows stacker.cmd
47b426c
phrohdoh pushed a commit to phrohdoh/stacker that referenced this pull request Dec 18, 2018
@ejholmes
Merge pull request cloudtools#532 from remind101/simple-logging
e2b4214 
Simplified logging
phrohdoh pushed a commit to phrohdoh/stacker that referenced this pull request Dec 18, 2018
@troyready @ejholmes
adapt cloudtools#532 to windows stacker.cmd
53c14c6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phobologic phobologic phobologic approved these changes

@russellballestrini russellballestrini russellballestrini approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.2

Development

Successfully merging this pull request may close these issues.

4 participants

@ejholmes @codecov-io @russellballestrini @phobologic