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

Revert PR #378 (Simplify NotebookNotary._data_dir_default) #397

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

krassowski
Copy link
Member

Reverts #378 to close #396.

This is because the breakage was also reported in the wild, see:

@ivanov ivanov added the bug label Mar 13, 2024
Copy link
Member

@ivanov ivanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm 👍 to reverting this for another patch release, since we currently have the latest patch release introducing this change, but I'd like to keep #396 open to think through alternatives or improvements. It just doesn't feel good to instantiate an app just for this one directory in cases where the NotebookNotary is being used by itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, can we take this file with whitespace changes out? it wasn't there in #378.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not make this, it was the precommit bot. I'm afraid I do not have permissions to override the bot

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry about that, @krassowski <3

@ivanov ivanov merged commit c17a251 into jupyter:main Mar 13, 2024
23 of 24 checks passed
@ivanov
Copy link
Member

ivanov commented Mar 13, 2024

I'd like to keep #396 open to think through alternatives or improvements

Nevermind that last part, the only remaining issue belongs in jupyter_core, so I opened jupyter/jupyter_core#397

@krassowski
Copy link
Member Author

Thanks for the merge! Another downstream affected: widgetti/ipyvuetify#301

@blink1073 a new patch release would be highly valued (once you have time to look at it)

@blink1073
Copy link
Contributor

Done!

@krassowski
Copy link
Member Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5.10.2 break server in certain circumstances (when data_dir was not yet created)
3 participants