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

Add support for "scope" option #284

Merged
merged 14 commits into from Oct 27, 2023
Merged

Add support for "scope" option #284

merged 14 commits into from Oct 27, 2023

Conversation

Bizordec
Copy link
Contributor

@Bizordec Bizordec commented Sep 3, 2023

Didn't find where to set the "scope" option, so here it is.

@2bndy5
Copy link
Collaborator

2bndy5 commented Sep 4, 2023

What does exposing this accomplish? I think the doc entry could be explained more. Is there info about this in the mkdocs-material theme?

@Bizordec
Copy link
Contributor Author

Bizordec commented Sep 4, 2023

It's useful in case when you build subsites separately. For example, for this structure

.
└── /
    ├── A/
    ├── B/
    └── C/

each site will have its own state for palette (/A/.palette, /B/.palette, /C/.palette). To share the same state between all subsites, you could set "scope": "/".

In mkdocs-material it's somehow still not documented, but here's the description in this issue: squidfunk/mkdocs-material#1914 (comment)

@2bndy5
Copy link
Collaborator

2bndy5 commented Sep 4, 2023

This seems useful for different versioned sets of docs. We do support custom cookies, but that's another feature that has gone undocumented here.

How have you tested this?

@Bizordec
Copy link
Contributor Author

Bizordec commented Sep 5, 2023

Added test and better description.

I've also created a PR in mkdocs-material (squidfunk/mkdocs-material#5967), guess it'll soon be documented there too 😃

2bndy5
2bndy5 previously requested changes Sep 6, 2023
tests/extra_scope_test.py Outdated Show resolved Hide resolved
docs/customization.rst Outdated Show resolved Hide resolved
Co-authored-by: Brendan <2bndy5@gmail.com>
tests/extra_scope_test.py Outdated Show resolved Hide resolved
tests/extra_scope_test.py Outdated Show resolved Hide resolved
@2bndy5
Copy link
Collaborator

2bndy5 commented Sep 8, 2023

I should add nox info to our contributing guidelines.

@2bndy5
Copy link
Collaborator

2bndy5 commented Sep 8, 2023

I've re-run the CI multiple times, but the docs build on the windows runner fails to download/cache the fonts from google's API:

http.client.RemoteDisconnected: Remote end closed connection without response

This is specific to this PR, but I just wanted to pass that along. 🤷🏼‍♂️ I'm starting to see this in other PR's CI runs...

@Bizordec
Copy link
Contributor Author

I think i've fixed that SSL problem by adding some pypi packages.
https://stackoverflow.com/a/46467942
https://github.com/Bizordec/sphinx-immaterial/actions/runs/6137800799

@2bndy5
Copy link
Collaborator

2bndy5 commented Sep 10, 2023

I was starting to suspect

  1. the google server was changed
  2. the Google API key we use to fetch fonts maybe outdated (🤷🏼‍♂️)

@Bizordec
Copy link
Contributor Author

Bizordec commented Sep 10, 2023

Okay, it failed again... I don't get it... Is it possible to somehow reset this runner?
Oh, wrong requirements.txt

@2bndy5
Copy link
Collaborator

2bndy5 commented Sep 10, 2023

Actually, I think the answer should be a short-lived cache of the fonts per CI run (specific to each OS). Currently, each unit test and docs build we do re-downloads the same sets of font families.

@jbms
Copy link
Owner

jbms commented Sep 12, 2023

We could cache if necessary but it does add some complications. The failing requests seem to be the plain requests to retrieve individual fonts. Those requests do not use an API key and occur anytime someone loads a page that uses Google fonts --- i.e. expected to get a lot of traffic.

I see that the CI is still failing, despite what was intended to fix it?

@jbms
Copy link
Owner

jbms commented Sep 12, 2023

Does it always fail with Python 3.11 on Windows, or is it random?

@2bndy5
Copy link
Collaborator

2bndy5 commented Sep 12, 2023

It seems to be specific to windows runner, but I've seen it happen on macos too. Caching the fonts only ensures the tests can exec without re-downloading the fonts (saves a few seconds per job) -- see #292 CI runs.

I think it is specific to py v3.11 too.

@Bizordec
Copy link
Contributor Author

And now windows build failed even before creating a font cache 😕

@2bndy5
Copy link
Collaborator

2bndy5 commented Sep 19, 2023

I've been starting to see the google fonts problem downstream in my sphinx-social-cards ext CI (specific to windows and mostly python v3.11).

In researching the possible problems, I noticed google fonts has a newer API to support variable fonts: https://developers.google.com/fonts/docs/css2. Could be worth a try (for fixed fonts), but I don't think variable fonts are supported in this theme.

@2bndy5
Copy link
Collaborator

2bndy5 commented Sep 19, 2023

Maybe this change in py3.11.5 might have something to do with the SSLEOFError we keep seeing on the windows runners. v3.11.5 was released Aug 24 2023.

@2bndy5
Copy link
Collaborator

2bndy5 commented Oct 4, 2023

I'm unable to reproduce this SSLError locally with Windows 11 + Python v3.11.5.

@Bizordec
Copy link
Contributor Author

Bizordec commented Oct 5, 2023

Python v3.11.6 just got released, maybe it will fix the issue
https://docs.python.org/release/3.11.6/whatsnew/changelog.html#python-3-11-6

Tools/Demos
gh-109991: Update GitHub CI workflows to use OpenSSL 3.0.11 and multissltests to use 1.1.1w, 3.0.11, and 3.1.3.

@2bndy5
Copy link
Collaborator

2bndy5 commented Oct 5, 2023

Unfortunately, the actions/setup-python still uses 3.11.5 as the latest.
image
I am eager to see if this was identified and fixed though.

Copy link
Collaborator

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

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

@jbms Any objections to merging this? I'd like to get this merged so I can push a tag and release the changes labeled "pending release"

@Bizordec Thank you so much for your patience and work (& rebasing) here!

@Bizordec
Copy link
Contributor Author

Bizordec commented Oct 5, 2023

Oh wow, build have finally passed! Although v3.11.5 was used 🤷‍♂️
Glad to help! 😄

@2bndy5
Copy link
Collaborator

2bndy5 commented Oct 9, 2023

It would seem that the github actions runners on windows have a cached version of python. Among the caches is python 3.11.5 (no later), so we either wait until the windows runner image is updated (takes a long time on purpose) or we explicitly tell actions/setup-python to download the latest version from github releases of actions/python-versions.

@2bndy5
Copy link
Collaborator

2bndy5 commented Oct 9, 2023

I still hit the SSLError in CI using python 3.11.6 on Windows (on a different repo using this theme in unit tests and docs builds). I think this might be more specific to google fonts API (probably a combination of py 3.11+ and google fonts API).

@2bndy5 2bndy5 requested a review from jbms October 23, 2023 21:23
@2bndy5
Copy link
Collaborator

2bndy5 commented Oct 27, 2023

actions/setup-python now uses v3.12.0 as the latest python version:
image

@jbms jbms merged commit 0d83cf3 into jbms:main Oct 27, 2023
57 checks passed
@2bndy5
Copy link
Collaborator

2bndy5 commented Oct 28, 2023

released with v0.11.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.

None yet

3 participants