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

Loading Jupytext configuration file messages #959

Closed
rcthomas opened this issue Jun 12, 2022 · 10 comments · Fixed by #963
Closed

Loading Jupytext configuration file messages #959

rcthomas opened this issue Jun 12, 2022 · 10 comments · Fixed by #963

Comments

@rcthomas
Copy link

I have a few notebooks open in JupyterLab with Jupytext 1.13.8, and I am noticing that about every 13 seconds it prints out about 10 times a message like:

[I 2022-06-12 08:12:06.306 SingleUserLabApp contentsmanager:528] Loading Jupytext configuration file at ...

This is pretty verbose, for me it adds about 1 MB of redundant output in 2 hours without really interacting with the notebooks. I've seen the previous discussions on issues and PRs where this has been brought up, so I understand that the idea is that it's good to let users know when a config file has been loaded. But I think the content being communicated here is most useful at debug level, and an info-level message would be something like "I've read the config and it's changed from the previous time it was loaded."

Or, is it possible I've got something configured wrong or another extension I have loaded is causing an increased frequency of these messages? Thanks!

@rcthomas
Copy link
Author

I thought about disabling the serverextension globally and having users opt in, but I find that I can't seem to disable it. Either that or it still loads the config file when it's disabled? When I do

    jupyter serverextension disable --sys-prefix jupytext

When starting up the notebook server I still see it loading the config. Doing jupyter serverextension list it's listed as disabled. And yet in my log I still see the extension loading and the config loading log messages.

@mwouts
Copy link
Owner

mwouts commented Jun 30, 2022

Hi @rcthomas , I have been working on a new option that should reduce very significantly the number of log lines regarding the config file. Would you like to give a try to the development version with e.g. this command? Thanks

BUILD_JUPYTERLAB_EXTENSION=1 pip install git+https://github.com/mwouts/jupytext.git@cm_config_log_level

@mwouts
Copy link
Owner

mwouts commented Jun 30, 2022

The PR #963 is ready - on the way I've had to handle the new allow_hidden=False parameters in jupyter_server==2.0.0a1, so when you test the above, make sure you don't use a hidden config file (i.e. use jupytext.toml, not .jupytext.toml).

@rcthomas
Copy link
Author

rcthomas commented Jul 1, 2022

Hi @mwouts thanks for looking into this, I've given it a go with a non-hidden config file. What I see now is that there is just the first loading configuration message, and no further ones. Thanks very much, this addresses the issue!

I looked at the PR and was a little surprised at what you're actually checking for as a change though --- if I read correctly you're looking at the config file name, not the config file metadata or the content. Is that what you really want to do (base the "change" on the filename itself and not the config content)? I could be reading that wrong but I don't think I am.

In any case this definitely helps cut down on the extra log messages, thank you again.

@mwouts
Copy link
Owner

mwouts commented Jul 1, 2022

Hi @rcthomas , thanks for the feedback, and for the suggestion - you mean something like 143a1be, don't you?

@rcthomas
Copy link
Author

rcthomas commented Jul 1, 2022

In principle yes, but I just tested and that change seems to bring back the old behavior even when the config file and its contents go unmodified.

The comparison config != prev_config is considering 2 objects of different types, config is a jupytext.config.JupytextConfiguration while prev_config is a jupytext.contentsmanager.cached_config --- so I think that comparison always comes up as True, right? Thanks!

@mwouts
Copy link
Owner

mwouts commented Jul 2, 2022

Thanks @rcthomas for testing! Oh yes sure I should have taken prev_config = ...cached_config.config - but even with that config != prev_config seems to always be True, which makes me think that the test that I added in the PR is not effective... I'll see what I can do.

@mwouts
Copy link
Owner

mwouts commented Jul 2, 2022

I've added the missing .config, the config comparison method, and revisited the test at 5d9a609 . If you can confirm that this works on your end as well it would be great! Thanks

@rcthomas
Copy link
Author

rcthomas commented Jul 2, 2022

I believe that did it! I tested changing the config live, and I only see the configuration loaded message when it's changed. Thank you so much for addressing this!

@mwouts
Copy link
Owner

mwouts commented Jul 3, 2022

Excellent! Thank you @rcthomas for testing this thoughtfully on your end, this is a very precious contribution. I'll integrate this in the next release (probably jupytext==1.14.0)

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

Successfully merging a pull request may close this issue.

2 participants