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

Enforce Python 3.6 in setup.py to support JKM #142

Merged
merged 5 commits into from
Dec 3, 2019

Conversation

echarles
Copy link
Member

@echarles echarles commented Dec 2, 2019

jupyter_kernel_mgmt 0.5.0 has just been released. It does not run on python 3.5.

This PR enforce a minimum version of 3.6 for Python.

Regarding EOL of 3.5:

Copy link
Contributor

@rolweber rolweber left a comment

Choose a reason for hiding this comment

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

lgtm

@rolweber
Copy link
Contributor

rolweber commented Dec 2, 2019

Strange - Travis builds with four Python versions, though only one appears in .travis.yml?
https://github.com/jupyter/jupyter_server/blob/master/.travis.yml

@rolweber
Copy link
Contributor

rolweber commented Dec 2, 2019

Correction: There's an explicit Matrix in .travis.yml:
https://github.com/jupyter/jupyter_server/blob/master/.travis.yml#L58-L65

But also a single specification for 3.6:
https://github.com/jupyter/jupyter_server/blob/master/.travis.yml#L8-L9

I suggest to update the matrix and to remove the single specification.
That should get rid of the breaking 3.5 Travis build.

@rolweber
Copy link
Contributor

rolweber commented Dec 2, 2019

The tests for Python 3.7 and 3.8 are failing in the nbconvert area:

ValueError: No template sub-directory with name 'classic' found in the following paths:
	/tmp/tmpekr6xwwp/data
	/tmp/tmpekr6xwwp/env/share/jupyter
	/tmp/tmpekr6xwwp/share/jupyter

@rolweber
Copy link
Contributor

rolweber commented Dec 2, 2019

It would make sense to enable Python 3.7 and 3.8 in AppVeyor, too.
https://github.com/jupyter/jupyter_server/blob/master/appveyor.yml#L6-L8

@takluyver
Copy link
Contributor

Just to mention, it wasn't entirely intentional that JKM dropped support for 3.5. If someone sees a clear reason to support it a bit longer, we can do that. But I'm certainly not complaining if we don't need it, and we can start using 3.6 features now.

@echarles
Copy link
Member Author

echarles commented Dec 2, 2019

Thx @rolweber for your review and advices. I have updated travis and appveyor specs based on your inputs. Let's see the results. The failing tests this is not linked to this PR. @Zsailer is working on jupyter_server_tests to make them more robust based on pytest and we can hopefully merge soon his work here.

@takluyver Thx to jump in. I hope we can merge this so we have a bit less issue to solve.

@echarles
Copy link
Member Author

echarles commented Dec 2, 2019

mmh CI settings seem to be ok but we have 72% to 73% success depending on py version.

The build history shows a lot of red which makes me think that the current noose test are flaky, which I have seen also on my laptop.

I would not worry on the returned red flag and let's see what @Zsailer can tell on this.

@rolweber
Copy link
Contributor

rolweber commented Dec 2, 2019

I think GROUP=docs should only be built with one Python version in Travis. The docs don't change with the Python version they're built on, right? And Travis asks that their free infrastructure shouldn't be burdened with pointless builds.

@echarles
Copy link
Member Author

echarles commented Dec 2, 2019

The 72% is taken from travis. appeveyor returns 100% success for 3.6, 3.7 and 3.8.

I have opened #143 to see if we agree to replace Travis with GitHub actions.

@echarles
Copy link
Member Author

echarles commented Dec 2, 2019

@rolweber yep, I was wondering on that while making the changes. On the other hand, if we e.g. create the API docs (sphinx autodoc), we want to be 100% sure that nothing is wrong. BTW see also my proposal to migrate from travis to github actions.

@kevin-bates
Copy link
Member

Although I haven't dug into things to confirm, I suspect the test failures (related to nbconvert) are due to a recently released version and we should probably port this fix: jupyter/notebook#5092

@echarles echarles merged commit 00428a1 into jupyter-server:master Dec 3, 2019
@echarles echarles deleted the py-3.6 branch December 3, 2019 10:03
@echarles
Copy link
Member Author

echarles commented Dec 3, 2019

@kevin-bates Thx for the investigation regarding the build failure. I have activated github actions for the ubuntu/macos/windos and py3.6/3.7/3.8 matrices and all is green

Travis is still showing errors with nbconvert but I suspect more an issue with server port management in the Travis Virtual Machine

@maartenbreddels
Copy link
Contributor

Just to mention, it wasn't entirely intentional that JKM dropped support for 3.5. If someone sees a clear reason to support it a bit longer

Actually, we (cc @SylvainCorlay) still did depend on python3.5.

@echarles
Copy link
Member Author

echarles commented Dec 4, 2019

@maartenbreddels What are your plans/timelines to drop 3.5 support?

@echarles
Copy link
Member Author

echarles commented Dec 4, 2019

@maartenbreddels just opened #150 to revert back to 3.5 for 0.2.0. We would only then remove 3.5 support for release 0.3.0.

Zsailer added a commit to Zsailer/jupyter_server that referenced this pull request Nov 18, 2022
* create a pipeline chain in RIO CI

* Bump to 0.12.8
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.

5 participants