Mike patch - #75
Mike patch#75
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a workaround for mike's documentation deployment tool to preserve custom fields in versions.json. The issue is that mike's VersionInfo class overwrites the entire versions.json file, removing fields that aren't part of its standard schema, which breaks the version selector for older Sphinx-based documentation sites. The solution monkey-patches mike's VersionInfo class to preserve these unknown fields during serialization and deserialization.
Key Changes:
- Added
_patch_versions_info()function that monkey-patches mike'sVersionInfoclass to preserve custom JSON fields - Created new
mike_deployinvoke task that wraps mike's deployment with the patching applied - Updated GitHub Actions workflow to use the new invoke task instead of calling mike directly
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tasks.py | Adds monkey-patching logic and new mike_deploy invoke task to preserve custom fields in versions.json when deploying documentation |
| .github/workflows/docs.yml | Updates documentation deployment workflow to use new invoke mike-deploy task instead of calling mike directly |
| run: | | ||
| git fetch origin gh-pages --depth=1 | ||
| mike deploy --push --update-aliases ${{ env.VERSION }} latest | ||
| invoke mike-deploy --version ${{ env.VERSION }} --push-to-origin true |
There was a problem hiding this comment.
The workflow passes --push-to-origin true as a string, but the mike_deploy function expects push_to_origin to be a bool. Invoke will interpret the string "true" as a truthy value and convert it to boolean True, so this should work. However, for clarity and explicitness, consider passing it without the value (as a flag) like --push-to-origin since the parameter defaults to False. Alternatively, if you want to be explicit, the current approach works but relies on invoke's automatic type conversion.
| invoke mike-deploy --version ${{ env.VERSION }} --push-to-origin true | |
| invoke mike-deploy --version ${{ env.VERSION }} --push-to-origin |
| VersionInfo.from_json = patched_from_json | ||
| VersionInfo.to_json = patched_to_json |
There was a problem hiding this comment.
The monkey-patch modifies the VersionInfo class globally for the entire process. If this function is called multiple times, the patching will occur multiple times, which is redundant but should not cause issues. However, consider adding a guard to prevent redundant patching, or document that this is intentional behavior. This would make the code more defensive and clearer about its intent.
|
|
||
| def _patch_versions_info(): | ||
| # our `versions.json` contains some fields which aren't supported by mike's | ||
| # VersionInfo class. Unfortunately mike overwrites the entire content everytime which breaks the version selector of the old Sphinx sites. |
There was a problem hiding this comment.
Typo in comment: "everytime" should be "every time" (two words).
| # VersionInfo class. Unfortunately mike overwrites the entire content everytime which breaks the version selector of the old Sphinx sites. | |
| # VersionInfo class. Unfortunately mike overwrites the entire content every time which breaks the version selector of the old Sphinx sites. |
|
|
||
|
|
||
| @task(help={"version": "The library version for which the documentation is to be deployed (e.g., '1.0.0')", "push_to_origin": "Whether to push the changes to the origin remote"}) | ||
| def mike_deploy(ctx, version: str, push_to_origin: bool = False): |
There was a problem hiding this comment.
The mike_deploy task lacks a docstring describing its purpose, parameters, and usage. According to the coding guidelines, public API functions and classes should have numpy style docstrings. While this is an invoke task and not strictly a public API function, it's part of the public interface exposed to developers. Adding a docstring would improve usability, especially since this task performs a non-trivial operation (monkey-patching).
| from compas_pb.invocations import proto_docs | ||
|
|
||
|
|
||
| def _patch_versions_info(): |
There was a problem hiding this comment.
The _patch_versions_info function lacks a docstring explaining what it does, why it's needed, and the side effects of monkey-patching. This is particularly important for a function that modifies third-party library behavior at runtime. A docstring would help future maintainers understand the purpose and implications of this patch.
| def _patch_versions_info(): | |
| def _patch_versions_info(): | |
| """Monkey-patch ``mike.versions.VersionInfo`` to preserve unknown JSON fields. | |
| This project stores additional metadata in ``versions.json`` which is not | |
| supported by mike's built-in :class:`~mike.versions.VersionInfo` model. | |
| By default, mike discards unknown fields on load and rewrites the file | |
| on dump, which would remove this extra metadata and break the version | |
| selector of existing Sphinx sites that rely on it. | |
| This helper replaces :meth:`VersionInfo.from_json` and | |
| :meth:`VersionInfo.to_json` at runtime so that: | |
| * unknown keys encountered during ``from_json`` are stored on the instance | |
| (via ``external_attrs``), and | |
| * those keys are merged back into the output of ``to_json``. | |
| Notes | |
| ----- | |
| This is a process-wide monkey-patch: once applied, all subsequent uses of | |
| :class:`mike.versions.VersionInfo` in the current process will use the | |
| patched serialization methods. Call this function before invoking any | |
| mike commands that load or write ``versions.json`` (for example, | |
| inside the ``mike_deploy`` task). | |
| """ |
| quiet=False, | ||
| ) | ||
|
|
||
| driver.deploy(None, args) |
There was a problem hiding this comment.
The driver.deploy(None, args) call passes None as the first argument. This should be verified to ensure it's the correct usage of the mike driver API. If None represents a missing configuration or context, it might be worth adding a comment explaining why this is acceptable or if there's a better value to pass.
gonzalocasas
left a comment
There was a problem hiding this comment.
Looks good! I think it's a good hack to get this going, perhaps moving forward, we should revert to use Mike without patching, and patch/tweak/update retroactively all the old version switcher
Here I'm shamelessly patching
mikeso that it doesn't overwrite our preciousversions.jsonthus breaking the version selector of the old Sphinx websites.What type of change is this?
Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.CHANGELOG.mdfile in theUnreleasedsection under the most fitting heading (e.g.Added,Changed,Removed).invoke test).invoke lint).compas.datastructures.Mesh.