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

Remove loop logger? #527

Closed
ejholmes opened this issue Feb 1, 2018 · 3 comments
Closed

Remove loop logger? #527

ejholmes opened this issue Feb 1, 2018 · 3 comments

Comments

@ejholmes
Copy link
Contributor

ejholmes commented Feb 1, 2018

The "loop logger" was originally added to try to make the log output a little more user friendly, however, having been using this on a large stacker.yaml for a while now, I feel like it hasn't really achieved that goal and hasn't scaled well.

There's a few problems with the loop logger:

  1. Since the status is ordered with dependencies first, on a large stack config you won't actually see any useful output until stacker reaches leaf nodes. So you're basically stuck seeing a bunch of "pending" messages for minutes on end.
  2. Ansi escape sequences don't allow for moving up past the top of the terminal. This means that after a stacker build on a large stacker config, if you scroll up you'll see a lot of cut off input.
  3. For me at least, it produces a lot of jitter when stacker writes out new statuses, since it has to write everything to the terminal again.
  4. When used with -i, you get a ton of useless log messages, since the entire list of steps is output at every status change.

I wonder if just a simple log message per status update would be better, and more scalable, in the long run. As an example, this is what I'm thinking:

asciicast

@phobologic
Copy link
Member

I think it's important to consider why the loop logger was implemented: we were having a lot of spam on screen even with 50 stacks that weren't getting updated.

That said, I think there are other ways to fix this, while just using a plain logger like we have here. The biggest being: don't show any messages unless a change is happening. So if a status would be "skipped" for any reason, we don't even show that we're submitting it in the usual usage. We could make --verbose more granular - a single -v shows every stack that it processes, but doesn't go into DEBUG logging, and two -vv does the current DEBUG logging. Or, if we're worried about mucking with an existing flag's functionality, we could have separate flag.

I think that'd fix the original issue (we have 300 stacks now? even the loop logger is useless at this point) and still give people the level of visibility they need.

@ejholmes
Copy link
Contributor Author

ejholmes commented Feb 1, 2018

That sounds pretty good to me. I think there's a possibility that it could create confusion if we display nothing for skipped stacks by default. In general, most updates maybe touch 1-2 stacks, so (at least on something as big as our stack config), you'd see no output for a while and might feel like stacker is doing nothing.

That may not really be a problem once the graph walk has ben parallelized though.

@phobologic
Copy link
Member

Yeah - I was wondering if that'd lead to confusion too, and was sort of why it wasn't the default behavior before. Maybe a small "spinny thing" to show that stacker is running? A modification of this where we can control the animation per tick? https://github.com/harshasrinivas/spinning/

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

No branches or pull requests

2 participants