-
Notifications
You must be signed in to change notification settings - Fork 403
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
docs-ci is failing with error from sphinx_markdown_tables extension #982
Comments
Also impacting the nextstrain/nextstrain.org repo's docs build. |
This is impacting all of our docs builds that have sphinx_markdown_tables as a dependency: n=7/9 from GitHub search. The 2 that don't have it are sphinx-theme and readthedocs-testbed, so functionally all of our docs are impacted. It's been a few days and I don't see any contributor activity on ryanfox/sphinx-markdown-tables#36, so not sure what the timeline is for a fix there. In the meantime, two approaches to (temp) fix this for ourselves:
1 is logically more proper since it puts the new pin next to the impacted dependency I'd lean towards (2) since it's easier to manage. Thoughts? |
I'm fine either way. In some sense it's still 7 docs projects to manage regardless of option 1 or 2 above. With option 2, for example, we'll have to trigger new builds for the "latest" version of each project so it picks up the new dep (or wait for this to happen in due course due to other dev). That said, option 2 does let us fix any "stable" builds that are busted because of this (if any?) by re-running them, which isn't possible with option 1.
FWIW, I don't think it's reasonable to expect any sort of activity/response over just a few days, esp. when those few days were a Friday, Saturday, and Sunday (and during prime summer holidays in the northern hemisphere no less). |
Fair enough! I don't mind letting things fail as long as we're aware of the issue and that we aren't able to push any new docs content until it's fixed. We could let this sit until ryanfox/sphinx-markdown-tables#36 gets fixed, or move forward with option 2 if anyone has new docs content that they'd really want to push. |
I don't expect this will be fixed soon, the last commit was 2y ago. Sure, maybe it all worked perfectly but my feeling is this will take at least a month to be solved if at all. May have to be forked and fixed by community if someone knows how to. May be simple, may not be. I'd be in favour of us PRing the fixes. Pinning shouldn't be too bad and this may happen again in the future. No activity for another 3d now. |
There seems to be a fork that has been patched. May be more sustainable unless we want to stick to markdown 3.3 forever. |
The fix is incredibly simple, a couple additional characters. On top of that, the whole extension is also very short (~40 lines). We could vendor it with minimal overhead, for example. But it's only been 3d total—not "another 3d"—and waiting a few more and/or working around as needed (with version pinning or vendoring) seems fine to me. I wouldn't suggest installing from some random patched repo, as that seems like the worst of all options (unknown author + code is pinned and will never see an update). |
…wn packages See commentary in this commit, the upstream issue¹, our initial tracking issue² (in another repo), and our Slack thread.³ ¹ ryanfox/sphinx-markdown-tables#36 ² nextstrain/ncov#982 ³ https://bedfordlab.slack.com/archives/C01LCTT7JNN/p1658148441308079
I implemented option 2 as nextstrain/sphinx-theme#29, released as nextstrain-sphinx-theme 2022.8. |
I've gone through all our RTD projects looking for failed builds due to this issue and retriggering them. |
Looks good, the PR CI seems to work now, thanks @tsibley ! |
Example run where docs-ci is failing.
This is due to ryanfox/sphinx-markdown-tables#36 and we will need to wait for that to be fixed.
The text was updated successfully, but these errors were encountered: