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

Docs not quite representative of the real "latest" version? #1946

Closed
christianaaronschroeder opened this issue May 6, 2024 · 10 comments · Fixed by #1965
Closed

Docs not quite representative of the real "latest" version? #1946

christianaaronschroeder opened this issue May 6, 2024 · 10 comments · Fixed by #1965
Labels
documentation Documentation about a certain topic should be added

Comments

@christianaaronschroeder
Copy link

This could be me misunderstanding what is meant by "latest" but I've noticed a few times that there is a feature that the docs imply is available in the "latest" release but is not actually available yet, rather it will be available in the next release.

E.g. A new plugin, TreeLayerControl, is now listed as part of the "latest" version in the docs. However, it was only merged back in April (#1895) and is not actually available in the "latest" release, 0.16.0.

@Conengmo
Copy link
Member

Conengmo commented May 6, 2024

That's a great point Christian, and I'd like to improve that. But I'm not sure how to do it in a sane way without too much manual labour on every release. Maybe you have some ideas?

I'd like it to be just like on https://pydata-sphinx-theme.readthedocs.io/en/stable/, with a version number with (latest) as default. And a dev option with the main branch docs. But since they use readthedocs I can't just copy their Github Actions settings.

Currently we have this Github Actions workflow for docs: https://github.com/python-visualization/folium/blob/main/.github/workflows/deploy-docs.yml. It does two things:

  • on a merge to main, push the docs to the existing latest folder on the gh-pages branch.
  • on a release, push the docs to a new folder with the version as folder name on the gh-pages branch.

Then on release we have to manually update the switcher.json file in our repo.

Note that our gh-pages branch has a file index.html which redirects to the latest folder, which is why that one is used by default.

I'd like it to be so that:

  • on a merge to main, update the dev folder on the gh-pages branch.
  • on a release:
    • update the switcher.json file in our repo (i.e. make a commit)
    • push the docs to a new folder on the gh-pages branch.

Updating the switcher.json file automatically is something I'm not sure how we can do that sensibly. Help is welcome!

@Conengmo Conengmo changed the title NIT: Docs not quite representative of the real "latest" version? Docs not quite representative of the real "latest" version? May 6, 2024
@Conengmo Conengmo added the documentation Documentation about a certain topic should be added label May 6, 2024
@merschformann
Copy link
Contributor

I stumbled upon this for the same reason as @christianaaronschroeder. At that note, when is the next release planned? 🥺

However, I took a stab at your request @Conengmo . Find it here (direct link to changes).

I didn't see a minimal way to achieve the requested functionality without introducing some potentially hard to maintain jq-fu, so, I created a small script that does the bump in switcher.json (which is hopefully easier to read).

The changes are intended to get pushed via stefanzweifel/git-auto-commit-action, but that won't work in the current form as main is a protected branch. There is a way of bypassing the protection by using a PAT created for specific actors allowed to override protection, but that feels unclean.
So, maybe it should instead open a PR automatically and request a review from specific people? That would also allow some manual intervention (some more info on this).

Let me know what you think. Happy to push this further if it makes any sense. 😊

@Conengmo
Copy link
Member

This is great Marius! Really happy with what you made so far. Making a Python script to update the switcher json file makes a lot of sense. It's easy to read. And your suggestion to open a PR automatically with the switcher change seems perfect to me. Hope you have the time to make a PR with this!

@merschformann
Copy link
Contributor

Happy to help. Also, thanks a lot for the great work on the project! It helped me a lot. 🤗

I will try to finish the PR tonight (I should have some time).

I will probably stick with automatically creating a PR and pinging for reviews. It's not fully automated that way, but less of a security concern. Happy about comments on this though. 😊

@merschformann
Copy link
Contributor

Sorry for sitting on this for longer than planned. 😬

I don't think there is an obvious solution to this, so, I want to list some options first as the decision should be made by the maintainers of the repository.

I played around with two options mainly and have others in mind (happy to look into them). I'll list them below:

  1. Use stefanzweifel/git-auto-commit-action@v5 in combination with a PAT (Personal Access Token, fine-grained) to push the changes directly to main from the workflow.
    • This requires a PAT to be configured by an account with admin access to the repository (as it needs to be able to bypass the branch protection rules that I can see are up).
    • The PAT needs to be stored as a secret in the repository (key: PAT).
    • The PAT needs "Contents" read/write access.
    • As this is security sensitive, please don't just take my word for it but also confirm with the action's docs on pushing to protected branches.
    • I tested this in a sandbox repository and it worked as expected.
    • Pros/cons:
      • More automated, no manual intervention needed.
      • PATs are not exactly elegant imho.
        • They expire by default and need to be manually rotated periodically.
  2. Create a new branch, commit the changes and create a PR to main.
    • This is another possibility, but it requires a manual review and merge of the PR.
    • Requires "Allow GitHub Actions to create and approve pull requests" to be enabled in the repository settings.
    • Pros/cons:
      • More manual work involved.
      • Does not require a PAT that needs to be manually rotated and handled securely.
  3. Use bot user to push changes to main branch
    • Another option that I used often in the past.
    • Requires a bot user account with admin access (to bypass branch protection rules).
    • Requires an SSH key (configured in the bot account) to be stored as a secret in the repository.
    • Pros/cons:
      • More automated, no manual intervention needed.
      • SSH keys don't expire like PATs (good and bad).
      • Requires a bot user account to be created and maintained.
      • Access of the bot can be managed like any other contributor.
  4. Short-lived tokens provided by a Github app
    • I have not used this before, but happy to look into it.
    • Similar to the first and third option, but with a different way of handling tokens.
    • Pros/cons:
      • More automated, no manual intervention needed.
      • Github app needs to be created and maintained.
      • No need to rotate tokens manually while also not having them live forever.

Let me know what you think. Happy to keep pushing this further.

The drafts of the first two options live here (option 1) and here (option 2). I played with them for a while in a sandbox repository here.

cc @Conengmo

@Conengmo
Copy link
Member

Conengmo commented Jun 2, 2024

Sorry for sitting on this for longer than planned

No worries! We're all volunteers, so no need to apologise! I'm very thankful you're taking this on!

Love your list of the options with explanation. Based on it, I'd say to go with option 2. The security risk of Actions being able to write directly makes me a bit uncomfortable. And having to rotate keys or maintain a bot/app sounds like more trouble than merging a PR. Plus we already have some bots that create pull requests, so it fits with a current way of working.

Does that sound like a reasonable choice?

Great that you also tested your code on an actual repo, that gives confidence to merge it. I'd say let's go for a PR and try a release after we merge it.

@merschformann
Copy link
Contributor

I opened #1965. Let me know whether it makes sense. Happy to update the PR. And please check me on the changes done to docs/conf.py and the updates to switcher.json. I ran some tests in my sandbox repo, but am not sure whether I missed something / fully grasped the doc generation. 😊

@Conengmo
Copy link
Member

We'll see how it goes and fix any issues if they come up. But I'm happy you tested in your sandbox repo already!

@Conengmo Conengmo linked a pull request Jun 14, 2024 that will close this issue
@Conengmo
Copy link
Member

I guess we should do a release to get the version dropdown in the docs to behave correctly, since they currently still get the data from the latest folder.

@merschformann
Copy link
Contributor

I guess we should do a release to get the version dropdown in the docs to behave correctly, since they currently still get the data from the latest folder.

That would be awesome anyway, as I am hesitating to put a git sha based dependency in another project where I am using the not yet released TreeLayerControl plugin (see here). ❤️

And let me know if I can be of further help. I should have some time during the upcoming nights. 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation about a certain topic should be added
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants