From 7c28f914bbf4f53a9146854d79994a9f76f42fa3 Mon Sep 17 00:00:00 2001 From: 12rambau Date: Fri, 7 Oct 2022 13:34:53 +0200 Subject: [PATCH 01/10] use warning instead of errors --- src/pydata_sphinx_theme/__init__.py | 40 +++++++++++++++++++---------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index 8f8043982..0ca7ea7d2 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -92,22 +92,34 @@ def update_config(app, env): # try to read the json file. If it's a url we use request, # else we simply read the local file from the source directory - # it will raise an error if the file does not exist + # display a log warning if the file cannot be reached + content = None if urlparse(json_url).scheme in ["http", "https"]: - content = requests.get(json_url).text + try: + content = requests.get(json_url).text + except ConnectionError: + logger.warning( + f'The version switcher "{json_url}" file cannot be read.' + ) else: - content = Path(env.srcdir, json_url).read_text() - - # check that the json file is not illformed - # it will throw an error if there is a an issue - switcher_content = json.loads(content) - missing_url = any(["url" not in e for e in switcher_content]) - missing_version = any(["version" not in e for e in switcher_content]) - if missing_url or missing_version: - raise AttributeError( - f'The version switcher "{json_url}" file is malformed' - ' at least one of the items is missing the "url" or "version" key' - ) + try: + content = Path(env.srcdir, json_url).read_text() + except FileNotFoundError: + logger.warning( + f'The version switcher "{json_url}" file cannot be read.' + ) + + # check that the json file is not illformed, + # throw a warning if the file is ill formed and an error if it's not json + if content is not None: + switcher_content = json.loads(content) + missing_url = any(["url" not in e for e in switcher_content]) + missing_version = any(["version" not in e for e in switcher_content]) + if missing_url or missing_version: + logger.warning( + f'The version switcher "{json_url}" file is malformed' + ' at least one of the items is missing the "url" or "version" key' + ) # Add an analytics ID to the site if provided analytics = theme_options.get("analytics", {}) From be85cdc5eb9880de75b0311511de07848956660f Mon Sep 17 00:00:00 2001 From: 12rambau Date: Fri, 7 Oct 2022 13:40:41 +0200 Subject: [PATCH 02/10] cleaning --- src/pydata_sphinx_theme/__init__.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index 0ca7ea7d2..be733956c 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -98,20 +98,18 @@ def update_config(app, env): try: content = requests.get(json_url).text except ConnectionError: - logger.warning( - f'The version switcher "{json_url}" file cannot be read.' - ) + pass else: try: content = Path(env.srcdir, json_url).read_text() except FileNotFoundError: - logger.warning( - f'The version switcher "{json_url}" file cannot be read.' - ) + pass - # check that the json file is not illformed, - # throw a warning if the file is ill formed and an error if it's not json - if content is not None: + if content is None: + logger.warning(f'The version switcher "{json_url}" file cannot be read.') + else: + # check that the json file is not illformed, + # throw a warning if the file is ill formed and an error if it's not json switcher_content = json.loads(content) missing_url = any(["url" not in e for e in switcher_content]) missing_version = any(["version" not in e for e in switcher_content]) From a866976906545f61cb4f07619dde8ee9cd989e43 Mon Sep 17 00:00:00 2001 From: 12rambau Date: Wed, 12 Oct 2022 14:42:44 +0200 Subject: [PATCH 03/10] catch error if the request returns 4xx or 5xx --- src/pydata_sphinx_theme/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index be733956c..4daacda9f 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -96,8 +96,10 @@ def update_config(app, env): content = None if urlparse(json_url).scheme in ["http", "https"]: try: - content = requests.get(json_url).text - except ConnectionError: + request = requests.get(json_url) + request.raise_for_status() + content = request.text + except (ConnectionError, requests.HTTPError): pass else: try: From 5faf8db24bc83a8bad58c0e44a8afb204adb5352 Mon Sep 17 00:00:00 2001 From: 12rambau Date: Thu, 13 Oct 2022 14:31:46 +0200 Subject: [PATCH 04/10] add some tests --- src/pydata_sphinx_theme/__init__.py | 3 ++- tests/sites/base/missing_url.json | 14 ++++++++++ tests/test_build.py | 42 +++++++++++++++++++++-------- 3 files changed, 47 insertions(+), 12 deletions(-) create mode 100644 tests/sites/base/missing_url.json diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index 4daacda9f..0c0d79784 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -21,6 +21,7 @@ from pygments.formatters import HtmlFormatter from pygments.styles import get_all_styles import requests +from requests.exceptions import ConnectionError, HTTPError, RetryError from .bootstrap_html_translator import BootstrapHTML5Translator @@ -99,7 +100,7 @@ def update_config(app, env): request = requests.get(json_url) request.raise_for_status() content = request.text - except (ConnectionError, requests.HTTPError): + except (ConnectionError, HTTPError, RetryError): pass else: try: diff --git a/tests/sites/base/missing_url.json b/tests/sites/base/missing_url.json new file mode 100644 index 000000000..b7e92ecca --- /dev/null +++ b/tests/sites/base/missing_url.json @@ -0,0 +1,14 @@ +[ + { + "name": "v0.7.1 (stable)", + "version": "0.7.1", + "url": "https://pydata-sphinx-theme.readthedocs.io/en/v0.7.1/" + }, + { + "version": "0.7.0", + "url": "https://pydata-sphinx-theme.readthedocs.io/en/v0.7.0/" + }, + { + "version": "0.6.3" + } +] diff --git a/tests/test_build.py b/tests/test_build.py index 9d1795d73..f069536e3 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -1,4 +1,5 @@ import os +import re from pathlib import Path from shutil import copytree @@ -11,6 +12,12 @@ path_tests = Path(__file__).parent +def escape_ansi(string): + """helper function to remove ansi coloring from sphinx warnings""" + ansi_escape = re.compile(r"(\x9B|\x1B\[)[0-?]*[ -\/]*[@-~]") + return ansi_escape.sub("", string) + + class SphinxBuild: def __init__(self, app: SphinxTestApp, src: Path): self.app = app @@ -628,7 +635,12 @@ def test_show_nav_level(sphinx_build_factory): assert "checked" in checkbox.attrs -def test_version_switcher(sphinx_build_factory, file_regression): +switcher_files = ["switcher.json", "http://a.b/switcher.json", "missing_url.json"] +"the switcher files tested in test_version_switcher, not all of them exist" + + +@pytest.mark.parametrize("url", switcher_files) +def test_version_switcher(sphinx_build_factory, file_regression, url): """Regression test the version switcher dropdown HTML. Note that a lot of the switcher HTML gets populated by JavaScript, @@ -641,20 +653,28 @@ def test_version_switcher(sphinx_build_factory, file_regression): "html_theme_options": { "navbar_end": ["version-switcher"], "switcher": { - "json_url": "switcher.json", + "json_url": url, "version_match": "0.7.1", }, } } - sphinx_build = sphinx_build_factory("base", confoverrides=confoverrides).build() - switcher = sphinx_build.html_tree("index.html").select( - ".version-switcher__container" - )[ - 0 - ] # noqa - file_regression.check( - switcher.prettify(), basename="navbar_switcher", extension=".html" - ) + factory = sphinx_build_factory("base", confoverrides=confoverrides) + sphinx_build = factory.build(no_warning=False) + + if url == "switcher.json": # this should work + index = sphinx_build.html_tree("index.html") + switcher = index.select(".version-switcher__container")[0] + file_regression.check( + switcher.prettify(), basename="navbar_switcher", extension=".html" + ) + + elif url == "http://a.b/switcher.json": # this file doesn't exist" + not_read = 'WARNING: The version switcher "http://a.b/switcher.json" file cannot be read.' # noqa + assert escape_ansi(sphinx_build.warnings).strip() == not_read + + elif url == "missing_url.json": # this file is missing the url key for one version + missing_url = 'WARNING: The version switcher "missing_url.json" file is malformed at least one of the items is missing the "url" or "version" key' # noqa + assert escape_ansi(sphinx_build.warnings).strip() == missing_url def test_theme_switcher(sphinx_build_factory, file_regression): From 96d5687ac45b846980c9178056f84d9b80e35741 Mon Sep 17 00:00:00 2001 From: 12rambau Date: Wed, 26 Oct 2022 22:13:50 +0200 Subject: [PATCH 05/10] add a parameter to prevent json test --- docs/user_guide/version-dropdown.rst | 8 ++++++++ src/pydata_sphinx_theme/__init__.py | 4 +++- .../theme/pydata_sphinx_theme/theme.conf | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/user_guide/version-dropdown.rst b/docs/user_guide/version-dropdown.rst index 0fb064cf4..eb7227e5b 100644 --- a/docs/user_guide/version-dropdown.rst +++ b/docs/user_guide/version-dropdown.rst @@ -115,6 +115,14 @@ a few different ways: } } +By default the theme is testing the :code:`.json` file provided and outputs warnings in the Sphinx build. If this test breaks the pipeline of your docs, the test can be disabled by configuring the :code:`check_switcher` parameter in :code:`conf.py`: + +.. code-block:: python + + html_theme_options = { + # ... + "check_switcher": False + } Configure ``switcher['version_match']`` --------------------------------------- diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index 0c0d79784..12dc8a838 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -84,7 +84,9 @@ def update_config(app, env): ) # check the validity of the theme swithcer file - if isinstance(theme_options.get("switcher"), dict): + is_dict = isinstance(theme_options.get("switcher"), dict) + is_tested = theme_options.get("check_switcher") is True + if is_dict and is_tested: theme_switcher = theme_options.get("switcher") # raise an error if one of these compulsory keys is missing diff --git a/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/theme.conf b/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/theme.conf index 22a4ae582..37218f216 100644 --- a/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/theme.conf +++ b/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/theme.conf @@ -36,6 +36,7 @@ primary_sidebar_end = sidebar-ethical-ads.html footer_items = copyright.html, sphinx-version.html secondary_sidebar_items = page-toc.html, searchbox.html, edit-this-page.html, sourcelink.html switcher = +check_switcher = True pygment_light_style = tango pygment_dark_style = monokai logo = From 6d21567ae2b8990759252c5e170c27a819e6e48f Mon Sep 17 00:00:00 2001 From: 12rambau Date: Wed, 26 Oct 2022 22:26:45 +0200 Subject: [PATCH 06/10] fix tests --- src/pydata_sphinx_theme/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index 12dc8a838..9374e1e3c 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -85,7 +85,7 @@ def update_config(app, env): # check the validity of the theme swithcer file is_dict = isinstance(theme_options.get("switcher"), dict) - is_tested = theme_options.get("check_switcher") is True + is_tested = not theme_options.get("check_switcher") is False if is_dict and is_tested: theme_switcher = theme_options.get("switcher") From 7c9b1b799b4fd5801e837ded0a4390cb365d1c50 Mon Sep 17 00:00:00 2001 From: Rambaud Pierrick <12rambau@users.noreply.github.com> Date: Fri, 4 Nov 2022 22:41:11 +0100 Subject: [PATCH 07/10] Update src/pydata_sphinx_theme/__init__.py Co-authored-by: Daniel McCloy --- src/pydata_sphinx_theme/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index 9374e1e3c..ec828cbf4 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -85,8 +85,8 @@ def update_config(app, env): # check the validity of the theme swithcer file is_dict = isinstance(theme_options.get("switcher"), dict) - is_tested = not theme_options.get("check_switcher") is False - if is_dict and is_tested: + should_test = theme_options.get("check_switcher", False) + if is_dict and should_test: theme_switcher = theme_options.get("switcher") # raise an error if one of these compulsory keys is missing From 1227b4883d53d5c48511ae19515c96a51daab1d2 Mon Sep 17 00:00:00 2001 From: Rambaud Pierrick <12rambau@users.noreply.github.com> Date: Fri, 4 Nov 2022 22:41:27 +0100 Subject: [PATCH 08/10] Update src/pydata_sphinx_theme/__init__.py Co-authored-by: Daniel McCloy --- src/pydata_sphinx_theme/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index ec828cbf4..ecfd7f1a8 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -83,7 +83,7 @@ def update_config(app, env): " Set version URLs in JSON directly." ) - # check the validity of the theme swithcer file + # check the validity of the theme switcher file is_dict = isinstance(theme_options.get("switcher"), dict) should_test = theme_options.get("check_switcher", False) if is_dict and should_test: From 8cbf7cc460abe2e1314284ea623e2d9926b473fb Mon Sep 17 00:00:00 2001 From: 12rambau Date: Sun, 6 Nov 2022 20:27:58 +0100 Subject: [PATCH 09/10] test: fix default behaviour --- src/pydata_sphinx_theme/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index ecfd7f1a8..59aecdef4 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -85,7 +85,7 @@ def update_config(app, env): # check the validity of the theme switcher file is_dict = isinstance(theme_options.get("switcher"), dict) - should_test = theme_options.get("check_switcher", False) + should_test = theme_options.get("check_switcher", True) if is_dict and should_test: theme_switcher = theme_options.get("switcher") From af525c497b394ba7c7ac5163ce1227de2804cf38 Mon Sep 17 00:00:00 2001 From: 12rambau Date: Sun, 6 Nov 2022 20:41:46 +0100 Subject: [PATCH 10/10] add the error in the logging message --- .flake8 | 9 +++++---- src/pydata_sphinx_theme/__init__.py | 17 ++++++++++------- tests/test_build.py | 4 ++-- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/.flake8 b/.flake8 index 0d9abf7dd..453bbd46a 100644 --- a/.flake8 +++ b/.flake8 @@ -1,6 +1,7 @@ [flake8] -max-line-length = 88 -ignore = - E203, # space before : (needed for how black formats slicing) - W503, # line break before binary operator +# E203: space before ":" | needed for how black formats slicing +# E501: line too long | Black take care of it +# W503: line break before binary operator | Black take care of it +# W605: invalid escape sequence | we escape specific characters for sphinx +ignore = E203, E501, W503, W605 exclude = setup.py,docs/conf.py,node_modules,docs,build,dist diff --git a/src/pydata_sphinx_theme/__init__.py b/src/pydata_sphinx_theme/__init__.py index 59aecdef4..e7984859f 100644 --- a/src/pydata_sphinx_theme/__init__.py +++ b/src/pydata_sphinx_theme/__init__.py @@ -96,22 +96,25 @@ def update_config(app, env): # try to read the json file. If it's a url we use request, # else we simply read the local file from the source directory # display a log warning if the file cannot be reached - content = None + reading_error = None if urlparse(json_url).scheme in ["http", "https"]: try: request = requests.get(json_url) request.raise_for_status() content = request.text - except (ConnectionError, HTTPError, RetryError): - pass + except (ConnectionError, HTTPError, RetryError) as e: + reading_error = repr(e) else: try: content = Path(env.srcdir, json_url).read_text() - except FileNotFoundError: - pass + except FileNotFoundError as e: + reading_error = repr(e) - if content is None: - logger.warning(f'The version switcher "{json_url}" file cannot be read.') + if reading_error is not None: + logger.warning( + f'The version switcher "{json_url}" file cannot be read due to the following error:\n' + f"{reading_error}" + ) else: # check that the json file is not illformed, # throw a warning if the file is ill formed and an error if it's not json diff --git a/tests/test_build.py b/tests/test_build.py index f069536e3..4c5930a96 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -669,8 +669,8 @@ def test_version_switcher(sphinx_build_factory, file_regression, url): ) elif url == "http://a.b/switcher.json": # this file doesn't exist" - not_read = 'WARNING: The version switcher "http://a.b/switcher.json" file cannot be read.' # noqa - assert escape_ansi(sphinx_build.warnings).strip() == not_read + not_read = 'WARNING: The version switcher "http://a.b/switcher.json" file cannot be read due to the following error:\n' # noqa + assert not_read in escape_ansi(sphinx_build.warnings).strip() elif url == "missing_url.json": # this file is missing the url key for one version missing_url = 'WARNING: The version switcher "missing_url.json" file is malformed at least one of the items is missing the "url" or "version" key' # noqa