-
-
Notifications
You must be signed in to change notification settings - Fork 636
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
Consume Console/Logger singleton/subsystems to prevent interleaving in @console_rules #6004
Comments
Have updated the description. This is blocked by #6571. |
This is unblocked! |
Starting this one. |
Since we want to (sometimes) pipe all logging through the UX by enqueuing it somewhere, we need the ability to flip a switch that will globally affect logging. While we could in theory have this switch and enqueuing exist on the Python side, it feels more natural to me to move logging to rust before/while doing this. So, my strategy for this ticket looks roughly as follows:
Using This implements a portion of the description: when this is implemented, we will not yet have a |
@stuhood that sounds well thought out to me. Something I'm a little confused on, who is the consumer of this API? You mention several ways we may want to do logging—log file, stderr, enqueuing, etc. Is the intent that Pants developers choose which of these 3 they want to do when implementing a particular goal? Or Pants users pass something like a CLI flag to set behavior? Something in between? |
@Eric-Arellano : The approaches mentioned above are implementation details below the See the high level design from https://docs.google.com/document/d/1Hn73YlhTPROlULTMa_3A-Fdv7hAPiHFII8rvXri5l7E/edit?usp=sharing for more info on what more explicit logging would look like, with a |
Sounds reasonable :) A few more moving parts than I'd like, but I can't point at any that I'd get rid of, so... |
@stuhood Google doc complains I need permission. Mind giving read access to all with link? |
It should be accessible to everyone on |
@Eric-Arellano : Were you able to see it using your |
@illicitonion and @blorente had some great discussion V2 logging in #7729 (comment) that I'm copying here.
|
… is finished (#7729) We want to add logging to `./pants test` when using V2 both as a matter of UI and to avoid Travis timeouts of not receiving input for 10+ minutes. Eventually, we should be doing this via a `Logging` singleton, similar to the `Console` singleton, per #6004. This acts as a temporary workaround in the meantime. ### Result ``` $ ./pants --no-v1 --v2 test tests/python/pants_test/util:strutil tests/python/pants_test/util:dirutil tests/python/pants_test/util:contextutil tests/python/pants_test/util:tarutil tests/python/pants_test/util:argutil tests/python/pants_test/util:collections 18:53:47 [INFO] Starting tests: tests/python/pants_test/util:strutil 18:53:47 [INFO] Starting tests: tests/python/pants_test/util:dirutil 18:53:47 [INFO] Starting tests: tests/python/pants_test/util:argutil 18:53:47 [INFO] Starting tests: tests/python/pants_test/util:contextutil 18:53:47 [INFO] Starting tests: tests/python/pants_test/util:tarutil 18:53:47 [INFO] Starting tests: tests/python/pants_test/util:collections 18:53:57 [INFO] Tests succeeded: tests/python/pants_test/util:argutil 18:53:59 [INFO] Tests succeeded: tests/python/pants_test/util:collections 18:53:59 [INFO] Tests succeeded: tests/python/pants_test/util:strutil 18:53:59 [INFO] Tests succeeded: tests/python/pants_test/util:tarutil 18:54:00 [INFO] Tests failed: tests/python/pants_test/util:contextutil 18:54:03 [INFO] Tests failed: tests/python/pants_test/util:dirutil tests/python/pants_test/util:strutil stdout: ============================= test session starts ============================== platform darwin -- Python 3.6.8, pytest-3.6.4, py-1.8.0, pluggy-0.7.1 rootdir: /Users/eric/DocsLocal/code/projects/pants/.pants.d/process-executionTYuLur, inifile: plugins: timeout-1.2.1, cov-2.4.0 collected 5 items pants_test/util/test_strutil.py ..... [100%] =========================== 5 passed in 0.07 seconds =========================== tests/python/pants_test/util:dirutil stdout: ... ```
Broken off a small self-contained part of this in #7906 |
@gshuflin fixed this a while back! Thanks Greg. |
breakout task from the v2 FE HLD here
After #6571 is completed, we will be in a situation where the
@rules
that feed@console_rule
s are rendered using a curses-like UI, but because we will be writing to stderr in an uncoordinated fashion, usage of the existing Console singleton from a @console_rule will cause interleaved output to stderr (ie, log messages will be-mixed-with/corrupt the ascii codes that position the cursor).To fix this, we should change the implementation of the
Console
singleton such that it always send to stdout, and eitherScheduler::execute
in Curses UX renders during @console_rule execution with interleaved console output #6571.Additionally, we should either:
Logger
intrinsic that an@rule
maySelect
that will be customized/prefixed for that@rule
logging
module) that happens within the externs that call arbitrary python code... and then render the output of logging the same way we render output from the
Console
singleton.The text was updated successfully, but these errors were encountered: