xtask: Use reqwest to fetch gen_init_cpio.c
#1208
Conversation
✅ Deploy Preview for aya-rs-docs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
dave-tucker
left a comment
dave-tucker
left a comment
There was a problem hiding this comment.
I understand the motivation that's driving this change, however, I'd rather keep functionality the same as the existing solution with octorust. See comments on how I feel this can be accomplished.
| headers: _, | ||
| body: octorust::types::ContentFile { mut content, .. }, | ||
| } = tokio::runtime::Builder::new_current_thread() | ||
| let content = tokio::runtime::Builder::new_current_thread() |
There was a problem hiding this comment.
I'd rather use: https://docs.rs/reqwest/latest/reqwest/blocking/index.html
| /// | ||
| /// This may be required if Github rate limits are exceeded. | ||
| #[clap(long)] | ||
| github_api_token: Option<String>, |
There was a problem hiding this comment.
This served a purpose. We frequently were getting rate limited in CI. Please reinstate it.
| "master", | ||
| )) | ||
| .block_on(async { | ||
| reqwest::get("https://github.com/torvalds/linux/raw/refs/heads/master/usr/gen_init_cpio.c") |
There was a problem hiding this comment.
You'll want to use https://docs.rs/reqwest/latest/reqwest/blocking/struct.Client.html
so we can set headers.
This doc suggests it would be wise to:
- Set the header for the expect API version
- Set the auth header if
--github-api-tokenwas provided
Furthermore, I would suggest using the same API endpoint as octorust was using due to the aforementioned issues with rate limiting.
https://docs.github.com/en/rest/repos/contents?apiVersion=2022-11-28
You'll then want to parse the reponse with serde_json
|
Alternatively we could wait for oxidecomputer/third-party-api-clients#99. |
|
Thanks for your review and comments. It looks like |
3 participants
Since
octorustdepends on an old version ofring, some newer Rust targets cannot compile with it. This patch switches to usingreqwestto fetchgen_init_cpio.cto make it work on more targets.This change is