Conversation
src/config.rs
Outdated
| let mut content_items = octocrab::instance() | ||
| .repos(repo.owner(), repo.name()) | ||
| .get_content() | ||
| .path(CONFIG_FILE_NAME) | ||
| .r#ref(&repo.default_branch) | ||
| .send() | ||
| .await | ||
| .map_err(|e| ConfigurationError::Http(Arc::new(e.into())))?; |
There was a problem hiding this comment.
Getting the file using the git API reduces the caching time but not completely. It looks like this way it's much shorter. Empirically (looking with RUST_LOG=trace) a couple of seconds, just until the connection is closed.
There was a problem hiding this comment.
IIUC this basically creates everytime a new HTTP client, not great, but this codepath is run just once at the start of triagebot and everytime the triagebot.toml of the rust repository is updated.
There was a problem hiding this comment.
I don't know yet what to think of REFRESH_EVERY_SECS.
There was a problem hiding this comment.
We can just pass the existing Context::octocrab instance to the get function to avoid creating a new client. Otherwise this looks fine. I guess that REFRESH_EVERY_SECS should work exactly the same as before?
There was a problem hiding this comment.
I did some more tests: my solution is not reliable, GH cache is just really sticky and I can't figure out a way around it. Certainly I can't have the pretension to outsmart them. I will leave this code here as a memento in case someone wants to try 🤷♂️
|
☔ The latest upstream changes (possibly #2344) made this pull request unmergeable. Please resolve the merge conflicts. |
@ehuss do you have thoughts about this approach?
(without looking too much at the messy code)