beam icon indicating copy to clipboard operation
beam copied to clipboard

Remove beam logging in playground for Python

Open hjtran opened this issue 1 year ago • 3 comments

I get a lot of complaints about all the logging in Beam playground for python examples. It really drowns out any output that might be coming from the examples.

I attempted to just remove the logging altogether for the python examples here, though I have no way of testing it.

My rationale is that the Beam playground environment doesn't really make sense to have logging for. If something goes wrong with an example, it makes a lot more sense to just copy and paste the example into your local Beam environment and debug it there (and optionally turn on logging in your own environment).

hjtran avatar Oct 10 '24 16:10 hjtran

R: @robertwb

hjtran avatar Oct 10 '24 16:10 hjtran

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

github-actions[bot] avatar Oct 10 '24 16:10 github-actions[bot]

R: @lostluck ?

hjtran avatar Oct 18 '24 15:10 hjtran

@liferoad - Who might be a good person to review this change.

My 2 cents: To be honest, I am not sure what is a good change here. People use playground to learn, and logs help learning. But too much logging does not really help with learning.

aaltay avatar Oct 21 '24 00:10 aaltay

cc @volatilemolotov

liferoad avatar Oct 21 '24 01:10 liferoad

@hjtran could you provide some examples to show the logging issue here?

liferoad avatar Oct 21 '24 01:10 liferoad

@hjtran @liferoad Yeah examples would be good. I agree with @aaltay on this one as logging does show some internals and could be valuable for learning the SDK. Especially if results are usually at the end of stdout and you can easily scroll to the end

volatilemolotov avatar Oct 21 '24 07:10 volatilemolotov

Hey all, thanks for the reviews!

Here's an example with AggregationSum image

The results of this example are taken from the Apache Beam Playground cache.
2024-10-02 15:17:04,163 [INFO] Using Any for unsupported type: typing.Sequence[~T]
2024-10-02 15:17:04,388 [INFO] Missing pipeline option (runner). Executing pipeline using the default runner: DirectRunner.
2024-10-02 15:17:04,574 [WARNING] Dependencies required for Interactive Beam PCollection visualization are not available, please use: `pip install apache-beam[interactive]` to install necessary dependencies to enable all data visualization features.
2024-10-02 15:17:04,574 [WARNING] You cannot use Interactive Beam features when you are not in an interactive environment such as a Jupyter notebook or ipython terminal.
2024-10-02 15:17:04,888 [INFO] Creating state cache with size 104857600
55
  • The first INFO message it's not really even clear which transform is causing this.
  • The second message is a bit distracting imo since I don't think you can even use a different runner in Playground and the user is likely here just to learn about combiners.
  • The first warning message is again imo a bit distracting as the likely user probably isn't interested in this interactive visualization from beam
  • The second warning message is again not relevant
  • The final INFO message is hard for a user to even know what it's referring to The final logged "55" is what actually relates to the example, but it is dwarfed by the irrelevant messages that come before it. I've gotten feedback that it's hard to tell that there's any output related to the example.

I've been using Playground as a way to teach people the Beam model and abstractions and I think this kind of logging makes it a lot harder to do that.

hjtran avatar Oct 21 '24 12:10 hjtran

@hjtran we do not have a way to control the logging levels now with Playground. With your PR, it will remove all the logs.

liferoad avatar Oct 21 '24 16:10 liferoad

@liferoad we could adjust the levels like the way #25692 does.

I think it'd be an improvement to adjust to ERROR, but I'd still argue that these logging messages don't help with the goal of learning with Playground so we may as well remove them.

hjtran avatar Oct 21 '24 16:10 hjtran

Can we at least adjust this PR to ERROR? I think it is needed if users play with the wrong code.

liferoad avatar Oct 21 '24 21:10 liferoad

Can we at least adjust this PR to ERROR? I think it is needed if users play with the wrong code.

Changed!

hjtran avatar Oct 22 '24 02:10 hjtran

https://github.com/apache/beam/actions/runs/11451588425/job/31878797880?pr=32740 I tried twice but both failed.

liferoad avatar Oct 22 '24 10:10 liferoad

Any guess as to why the "Operation was canceled"? I'm not sure how to troubleshoot this

hjtran avatar Oct 22 '24 10:10 hjtran

Any guess as to why the "Operation was canceled"? I'm not sure how to troubleshoot this

Does this run locally? :playground:backend:runLint

liferoad avatar Oct 22 '24 11:10 liferoad

FWIW, I totally agree that these logs are quite unhelpful, regardless of if you're in the playground or not.

robertwb avatar Oct 29 '24 21:10 robertwb

How do I retrigger the precommit here? Looks like @volatilemolotov fixed the issue that caused the cancellation of the precommit runs

hjtran avatar Oct 31 '24 22:10 hjtran

Run Playground PreCommit

volatilemolotov avatar Oct 31 '24 22:10 volatilemolotov

@hjtran It will not be able to trigger by comment now as the PR is before my updates. Easiest way is to just add a new commit and push to re-trigger.

volatilemolotov avatar Oct 31 '24 22:10 volatilemolotov

Bump @liferoad (note, I'm away from computer for the month of November after today)

hjtran avatar Nov 04 '24 13:11 hjtran