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

Replaced execute preprocessor internals with nbclient calls #1211

Merged
merged 2 commits into from
Apr 1, 2020

Conversation

MSeal
Copy link
Contributor

@MSeal MSeal commented Mar 9, 2020

Finishes #821. Took me a while to get around to it, but here's the PR with all the stuff moved into nbclient deleted.

I found that given how light the wrapper is now only a few very basic tests should be present. I didn't like how I had to subprocess NBClient's class to account for all the traitlet assignment junk, but it works and leave the interface backwards compatible. If nbclient ever needs to move it's interface forward or off trailets or whatever, we can capture it here for translation against future versions.

@MSeal
Copy link
Contributor Author

MSeal commented Mar 9, 2020

I think after this merges we can do 6.0.0a1 release

@choldgraf
Copy link
Contributor

The amount red in this PR makes me happy

@MSeal
Copy link
Contributor Author

MSeal commented Mar 9, 2020

Not sure why 3.5 is hanging on allow-errors -- will debug that later

@davidbrochart
Copy link
Member

I tested this PR locally with python3.5, it seems to randomly hang on test_allow_errors (it doesn't happen with python3.6).
Asyncio was still missing some important features in 3.5, and nbclient is implemented using asyncio. We work around the limitations of 3.5 with e.g. the async_generator library, but there might be other things that are problematic.
Is there a plan to drop python3.5 in nbconvert, nbclient, and in the Jupyter ecosystem in general?
In Voila, switching to async means dropping python3.5 anyway, since we need async generators in Jinja, which is not going to happen in python3.5: pallets/jinja#1058

@choldgraf
Copy link
Contributor

choldgraf commented Mar 13, 2020

I am +1 on deprecating 3.5 after a deprecation cycle, now that we are already nearing 3.9 (obv the broader jupyter ecosystem will need to have a community decision on that)

@MSeal
Copy link
Contributor Author

MSeal commented Mar 19, 2020

Yeah the general guidance I've been following is to support 3.5 until it cycles out of the supported path for python proper. The reason I've tried to keep maintaining it is there's some larger orgs whose internal default python version is still 3.5. Dropping support for it before python does would disrupt updates for those orgs. Still I think later this year 3.5 can drop. For now I'd be ok with marking those tests as 3.6+ only and putting a big warning in the release notes that 3.5 may have stability issues with this change given async support. Thoughts?

@davidbrochart
Copy link
Member

Sounds good to me.

@MSeal MSeal force-pushed the replaceExecutorWithNbClient branch from 60367b2 to e7bf835 Compare March 26, 2020 19:06
@MSeal
Copy link
Contributor Author

MSeal commented Mar 31, 2020

Ok based on conversations across a few companies and in jupyter/nbclient#34 I'm going to update this PR and drop support for Python 3.5. The async code in upstream libraries has caused issues in each repo for python 3.5.

@MSeal
Copy link
Contributor Author

MSeal commented Apr 1, 2020

Going to merge this to get some things moving

@MSeal MSeal merged commit 0b42aaf into jupyter:master Apr 1, 2020
@MSeal MSeal added this to the 6.0 milestone Sep 8, 2020
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.

3 participants