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

Notebooks hangs when maxlog is used in logging and includes maxlog in output #1902

Closed
Joel-Dahne opened this issue Feb 8, 2022 · 2 comments · Fixed by #1911
Closed

Notebooks hangs when maxlog is used in logging and includes maxlog in output #1902

Joel-Dahne opened this issue Feb 8, 2022 · 2 comments · Fixed by #1911
Labels
bug Something isn't working logging About `@info`, `@warn`, etc

Comments

@Joel-Dahne
Copy link
Contributor

Joel-Dahne commented Feb 8, 2022

pluto-maxlog-stuck.mp4

Support for maxlog in logging was recently added in #1877. However I have issues with Pluto getting stuck on the second run of a cell with maxlog specified. In the video I stop after 10 seconds, but it doesn't finish after that either. Link to notebook.

One part of the code added in #1877 was

maybe_max_log = findfirst(((key, _),) -> key == "maxlog", next_log["kwargs"])
if maybe_max_log !== nothing
    n_logs = count(log -> log["id"] == next_log["id"], running_cell.logs)
    try
        max_log = parse(Int, next_log["kwargs"][maybe_max_log][2] |> first)

        # Don't show message with id more than max_log times
        if max_log isa Int && n_logs >= max_log
            return
        end
    catch
    end
end

I was able to get it to work for be by changing the return to continue. However I don't understand enough of Pluto's code to know if this is the proper fix.

I would also propose to follow the default for logging in Julia and not include the maxlog argument in the log message

julia> for i in 1:10
           @info "Some info" i maxlog = 2
       end
┌ Info: Some info
└   i = 1
┌ Info: Some info
└   i = 2

I was able to get this behavior by adding the line

deleteat!(next_log["kwargs"], maybe_max_log)

inside the try-statement. But again I don't know enough about Pluto to know if editing the log-message is allowed.

And thank you for adding support for logging! I think it can be very helpful!

@Pangoraw
Copy link
Collaborator

Pangoraw commented Feb 9, 2022

Good catch! Don't hesitate to open a pull request with your changes;

@Joel-Dahne
Copy link
Contributor Author

I can try to prepare a PR, possibly later today!

@fonsp fonsp added the bug Something isn't working label Feb 9, 2022
Joel-Dahne added a commit to Joel-Dahne/Pluto.jl that referenced this issue Feb 10, 2022
The previous version would hang of there were multiple logs and one of
them had maxlog set.
@fonsp fonsp closed this as completed in 63748e1 Feb 15, 2022
@fonsp fonsp added the logging About `@info`, `@warn`, etc label Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working logging About `@info`, `@warn`, etc
Projects
None yet
3 participants