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

Bump minimum version of jQuery to 3.6.0 #6471

Merged
merged 1 commit into from
Jul 28, 2022
Merged

Bump minimum version of jQuery to 3.6.0 #6471

merged 1 commit into from
Jul 28, 2022

Conversation

dylanraws
Copy link

Resolves #6469

@dylanraws
Copy link
Author

Tagging @RRosio as suggested by jweill-aws.

@echarles
Copy link
Member

Closing / Reopening to retrigger CI.

@echarles echarles closed this Jul 15, 2022
@echarles echarles reopened this Jul 15, 2022
@echarles
Copy link
Member

Looks like only 3 CI jobs have run. Is this expected?

@echarles
Copy link
Member

@dylanramzn I think the CI was previously run on Travis which has probably been desactived. Do you think that you could backport the GitHub actions used in the 6.5.x branch ? https://github.com/jupyter/notebook/tree/6.5.x/.github/workflows

@echarles
Copy link
Member

We have discussed this at the last community call with @jweill-aws. There has been a consensus to merge an release without a running CI, assuming the minimal sanity checks are done on a local env. This exceptional procedure is applied based on the nature of the change (jQuery front-end dependency upgrade being used all over the world).

I have tested locally this branch and as a user it works fine. I have also run the pytest command which returns 4 failed, 244 passed, 1 skipped, 1 xfailed, 71 warnings, 7 errors in 32.19s and was not able to successfully run any javascript test. The js failures are repicable on the vanilla 5.7.x branch. We could invest more time to make the javascript test work again but I wonder if this is worth to do that as we also said during the meeting that this will be the last patch accepted on this 5.7.x. branch.

python -m notebook.jstest base

Test group: base
Running tests with notebook directory '/var/folders/ly/vnyntf3n7q76382_rymt0qbm0000gn/T/tmp21ec1_8p'
Test file: /Users/echarles/Desktop/notebook-5.7.x/notebook/tests/base/highlight.js
Timeout for http://localhost:8888/a@b/
Is the notebook server running?
FAIL ".CodeMirror-code" still did not exist in 10000ms
#    type: uncaughtError
#    file: /Users/echarles/Desktop/notebook-5.7.x/notebook/tests/base/highlight.js
#    error: ".CodeMirror-code" still did not exist in 10000ms

#    stack: not provided

@dylanramzn BTW the latest jquery version is 3.6.0 and you are using here 3.5.0. Any reason for that?

@RRosio
Copy link
Collaborator

RRosio commented Jul 22, 2022

@dylanramzn Another thing to add is that pinning the dependency, jinja2 to a version <= 3.0.0 is necessary, as the more recent versions of jinja2 package cause import errors. I've verified this with a clean environment and a source package install which currently installs the most recent jinja2 version 3.1.2

Current Output with Jinja2==3.1.2
500: Internal Server Error

[E 08:04:41.149 NotebookApp] Uncaught exception GET /notebooks/getting-familiar-jnb.ipynb (::1)
    HTTPServerRequest(protocol='http', host='localhost:8888', method='GET', uri='/notebooks/getting-familiar-jnb.ipynb', version='HTTP/1.1', remote_ip='::1')
    Traceback (most recent call last):
      File "/Users/rosioreyes/miniconda3/envs/check_jQuery/lib/python3.8/site-packages/tornado/web.py", line 1711, in _execute
        result = method(*self.path_args, **self.path_kwargs)
      File "/Users/rosioreyes/miniconda3/envs/check_jQuery/lib/python3.8/site-packages/tornado/web.py", line 3208, in wrapper
        return method(self, *args, **kwargs)
      File "/Users/rosioreyes/Desktop/code/jupyter/notebook5/notebook/notebook/notebook/handlers.py", line 90, in get
        self.write(self.render_template('notebook.html',
      File "/Users/rosioreyes/Desktop/code/jupyter/notebook5/notebook/notebook/base/handlers.py", line 519, in render_template
        return template.render(**ns)
      File "/Users/rosioreyes/miniconda3/envs/check_jQuery/lib/python3.8/site-packages/jinja2/environment.py", line 1301, in render
        self.environment.handle_exception()
      File "/Users/rosioreyes/miniconda3/envs/check_jQuery/lib/python3.8/site-packages/jinja2/environment.py", line 936, in handle_exception
        raise rewrite_traceback_stack(source=source)
      File "/Users/rosioreyes/Desktop/code/jupyter/notebook5/notebook/notebook/templates/notebook.html", line 1, in top-level template code
        {% extends "page.html" %}
      File "/Users/rosioreyes/Desktop/code/jupyter/notebook5/notebook/notebook/templates/page.html", line 154, in top-level template code
        {% block header %}
      File "/Users/rosioreyes/Desktop/code/jupyter/notebook5/notebook/notebook/templates/notebook.html", line 112, in block 'header'
        {% for exporter in get_frontend_exporters() %}
      File "/Users/rosioreyes/Desktop/code/jupyter/notebook5/notebook/notebook/notebook/handlers.py", line 19, in get_frontend_exporters
        from nbconvert.exporters.base import get_export_names, get_exporter
      File "/Users/rosioreyes/miniconda3/envs/check_jQuery/lib/python3.8/site-packages/nbconvert/__init__.py", line 4, in 
        from .exporters import *
      File "/Users/rosioreyes/miniconda3/envs/check_jQuery/lib/python3.8/site-packages/nbconvert/exporters/__init__.py", line 3, in 
        from .html import HTMLExporter
      File "/Users/rosioreyes/miniconda3/envs/check_jQuery/lib/python3.8/site-packages/nbconvert/exporters/html.py", line 12, in 
        from jinja2 import contextfilter
    ImportError: cannot import name 'contextfilter' from 'jinja2' (/Users/rosioreyes/miniconda3/envs/check_jQuery/lib/python3.8/site-packages/jinja2/__init__.py)

Also pin jinja2<=3.0.0 to fix errors opening notebooks
@dylanraws dylanraws changed the title Bump minimum version of jQuery to 3.5.0 Bump minimum version of jQuery to 3.6.0 Jul 25, 2022
@dylanraws
Copy link
Author

BTW the latest jquery version is 3.6.0 and you are using here 3.5.0. Any reason for that?

This PR was originally just a back port of the commit made previously for Notebook 6. #5491

@RRosio I changed it to jQuery 3.6.0, and pinned jinja2 to <=3.0.0. I ran a sanity test and verified I can open a notebook and execute 1+1.

@RRosio
Copy link
Collaborator

RRosio commented Jul 28, 2022

This PR looks good so I will go ahead and merge!

@RRosio RRosio merged commit a02fe35 into jupyter:5.7.x Jul 28, 2022
@dylanraws dylanraws deleted the jquery-upgrade branch August 11, 2022 00:46
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants