The Wayback Machine - https://web.archive.org/web/20221214001648/https://github.com/byrnereese/mkdocs-git-committers-plugin/issues/12
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

Cache data fetched from GitHub between builds #12

Open
squidfunk opened this issue May 28, 2022 · 15 comments
Open

Cache data fetched from GitHub between builds #12

squidfunk opened this issue May 28, 2022 · 15 comments

Comments

@squidfunk
Copy link
Collaborator

squidfunk commented May 28, 2022

First of all, great plugin! I'm evaluating adding native support for this plugin to Material for MkDocs. The main challenge is the increase in build times – they are pretty significant. For instance, the build time for Material for MkDocs went up from 10s to 77s. Would it be possible to cache results?

The plugin would only need to cache the list of contributors per file + the information obtained for each contributor. If this data is normalized, fetching contributor information should only be necessary when there's a new contributor that hasn't been seen before. Other than that, the data is pretty much static – login, name, avatar URL. The name could change, but I'd suspect this is rarely the case. Also, for a page, the data would only need to be re-fetched when the commit SHA changed.

Happy to help.

@squidfunk squidfunk changed the title Cache results between builds Cache data fetched from GitHub between builds May 28, 2022
@ojacques
Copy link
Collaborator

ojacques commented May 31, 2022

@squidfunk, I hate to hijack this issue, but performance is one big reason I had to fork this project to create my own (I really dislike forking, but had to).
The fork adds cache, tries to get as much info as possible from local git repo and tries hard to reduce hits on GitHub's API. It also uses GitHub's GraphQL API for efficiency.

https://github.com/ojacques/mkdocs-git-committers-plugin-2

I have happily used it with mkdocs-material, although an old version. Happy to help. Also happy to get my changes in this upstream repo.

@squidfunk
Copy link
Collaborator Author

squidfunk commented May 31, 2022

@ojacques thanks! I've test-driven your project, but unfortuntely it produced inconsistent results for me. For example, my account (@squidfunk) was not correctly determined (login was empty). Maybe you can try it on the Material for MkDocs repository and track it down. I'm currently working on a tighter GitHub integration for this plugin and the git-revision-date(-localized) plugin.

@ojacques
Copy link
Collaborator

ojacques commented May 31, 2022

Maybe you can try it on the Material for MkDocs repository and track it down. I'm currently working on a tighter GitHub integration for this plugin and the git-revision-date(-localized) plugin.

Yes, I'll have a look. If this could be part of material proper, it would help me a ton. The short video is awesome (although I loved having contributors on top and not at the bottom).

@ojacques
Copy link
Collaborator

ojacques commented May 31, 2022

@squidfunk - after testing against mkdocs-material, I pushed a new version (0.4.0), which:

  • handles more (all?) cases
  • remove duplicated authors even if GIT authentication was different

It does not cache between builds though, but makes sure to only query info for a user once. To be precise, it will try first to get the info with the email then the name if not found, then the GitHub username if still not found.

Caching between builds is not there yet, but I can work on it by saving the dictionary file at the end of site generation and attempting to load it if it exists at the beginning.

@ojacques
Copy link
Collaborator

ojacques commented May 31, 2022

Caching between builds is not there yet, but I can work on it by saving the dictionary file at the end of site generation and attempting to load it if it exists at the beginning.

Done in 0.4.1. Although I'm not sure this is the best way/place to save this cache file. There is a risk for it to be committed if not added as part of .gitignore.

But hey, mkdocs-material build with authors is now 7 seconds for me, after the first run!

@squidfunk
Copy link
Collaborator Author

squidfunk commented Jun 1, 2022

Awesome, thanks for investing the time! I'll check it out the next time I work on this enhancement. I just put files to cache into a folder called .cache when I write plugins, e.g. for the built-in privacy plugin.

Yes, I'll have a look. If this could be part of material proper, it would help me a ton. The short video is awesome (although I loved having contributors on top and not at the bottom).

You will be able to easily change the location by overriding the content partial which controls the content area of the document. You'd just need to move the section to the top.

@squidfunk
Copy link
Collaborator Author

squidfunk commented Jun 18, 2022

I can confirm that the build time went waaaaay down, awesome job 🚀 I'll now consider it safe to recommend this plugin! The improved git plugin integration will be released in the coming weeks, I'll put a link to your plugin in our documentation.

One more thing: could we maybe make the cache folder configurable? I'd like to keep my .gitignore as short as possible, which is why I'd want to save authors.json to .cache. Could we maybe add a config option for that?

@byrnereese sorry for hi-jacking this issue, we should really have discussed this over at the fork.

Edit: Material for MkDocs' plugins will now write their caches into subfolders. Maybe this is also something to consider for the mkdocs-git-committers-plugin. We're currently using:

  • .cache/plugins/privacy
  • .cache/plugins/social

We recommend caching on our docs, which makes use of the .cache folder. To limit confusion, it might be a good idea to settle on that name, but it's certainly not necessary. If a setting would be available, we can recommend setting it to .cache/plugin/git-committers on our documentation 😊

@byrnereese
Copy link
Owner

byrnereese commented Jun 21, 2022

Why don't you issue a PR so we can stay in sync. No need to fork if we are all aligned.

@ojacques
Copy link
Collaborator

ojacques commented Jun 21, 2022

@byrnereese Absolutely. I will issue a PR against this repo. It may be big / hard to review. I apologize in advance.
Also, I got contributions lately against my fork improving this even further.
@squidfunk - I will expose the cache location as config, defaulting to the one you suggested above.

@squidfunk
Copy link
Collaborator Author

squidfunk commented Jun 22, 2022

@byrnereese your plugin looks a little unmaintained, as there are three open issues without any answers and there's not much commit activity. I guess this is also why the fork exists. If we could gravitate towards one solution, that would be awesome, but we'd need to be able to push out bug fixes fast.

I'd expect some increase on this or @ojacques' repository, depending on what we recommend using on our docs, so the moment this natively is supported by Material for MkDocs, there's also likely to be more exposure for your plugins, more users, and with that potentially more bugs and edge cases. The question is whether you're up for maintaining it.

However, consolidation of efforts would be something to strive for.

@byrnereese
Copy link
Owner

byrnereese commented Jun 23, 2022

I can certainly help when I am able. Happy to make you maintainer on this repository if that would give you more access, control and autonomy.

This is embarrassing, but for some reason I don't always get notified via email of new issues or PRs. Never figured out why. I will look into that so that I can be more aware of issues that come in.

@squidfunk
Copy link
Collaborator Author

squidfunk commented Jun 23, 2022

@byrnereese you can customize notification settings per repository here:

Bildschirmfoto 2022-06-23 um 07 56 48

@squidfunk
Copy link
Collaborator Author

squidfunk commented Jun 24, 2022

@ojacques @byrnereese insiders-4.19.0 adds native support for the git-authors and git-committers plugins. We currently recommend using @ojacques' fork because it implements caching. Otherwise, build times would skyrocket. If in the future, you both decide to merge efforts, please consider PR'ing a change to our documentation 😊 Here's how it looks:

Bildschirmfoto 2022-06-24 um 15 33 13

@ojacques
Copy link
Collaborator

ojacques commented Jun 24, 2022

This is so cool! Getting contributors listed in pages has been a big incentive for readers to be turned into contributors.

I will look at the first issue reported and continue to submit changes back to upstream.

@byrnereese - once / if you agree, happy to be made a maintainer of this repo and continue working with you.

@timvink
Copy link
Contributor

timvink commented Jun 24, 2022

Awesome work @squidfunk and @ojacques , really cool stuff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants