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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to jupyter_server 2.11.0 API #1145

Merged
merged 2 commits into from
Nov 27, 2023

Conversation

marr75
Copy link
Contributor

@marr75 marr75 commented Nov 22, 2023

Added md5 parameter to PloomberContentsManager.get and passed it along to super calls.

Describe your changes

Small interface changes to adapt to 2.11.0 jupyter_server changes

Issue number

Closes #1144

Checklist before requesting a review

  • Performed a self-review of my code
  • Formatted my code with pkgmt format

馃摎 Documentation preview 馃摎: https://ploomber--1145.org.readthedocs.build/en/1145/

Added md5 parameter to PloomberContentsManager.get and passed it along to super calls.
Copy link

Binder 馃憟 Launch a binder notebook on branch marr75/ploomber/#1144-jupyter_server-2.11.0

@marr75
Copy link
Contributor Author

marr75 commented Nov 22, 2023

I will add another commit to support <2.11 and >2.11

@edublancas
Copy link
Contributor

@marr75 thanks for working on this!

how are you thinking of supporting both versions? if the code gets too complicated, we can just require >2.11 for the next ploomber release

@Wh1isper
Copy link

Wh1isper commented Nov 23, 2023

Jupyter server 2.11.0 has been yanked, I'm trying to make this change compatible: jupyter-server/jupyter_server#1367

This may require you to make additional changes. Sorry to bring this up...

jupyter_server committers are looking to support variable hash algorithms so the variable name has changed.

Introducing *args and **kwargs and forwarding them to super should make this work with <2.11 and somewhat future proof the signature.
Copy link

Binder 馃憟 Launch a binder notebook on branch marr75/ploomber/#1144-jupyter_server-2.11.0

@marr75
Copy link
Contributor Author

marr75 commented Nov 25, 2023

I went with something simpler. Just forwarding *args and **kwargs to the super method.

@marr75
Copy link
Contributor Author

marr75 commented Nov 25, 2023

Tested with jupyter_server 2.10 and 2.12.dev0

@marr75
Copy link
Contributor Author

marr75 commented Nov 25, 2023

@Wh1isper thanks for the heads up, the 2.12 change ended up spurring a simpler change from me anyway!

@Wh1isper
Copy link

Glad to hear it!

@edublancas edublancas merged commit bdce5bc into ploomber:master Nov 27, 2023
11 of 13 checks passed
@edublancas
Copy link
Contributor

@marr75 thanks a lot for your contribution! there's some minor CI issue we have fix (unrelated to this change), once we fix it, we'll release this. in the meantime, you can install from git

@edublancas
Copy link
Contributor

@marr75 this has been released in version 0.23.1!

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 this pull request may close these issues.

jupyter_server 2.11.0 introduces breaking change to contents_manager.get
3 participants