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

Dynamic quarto #176

Merged
merged 6 commits into from
May 21, 2024
Merged

Dynamic quarto #176

merged 6 commits into from
May 21, 2024

Conversation

samcofer
Copy link
Contributor

@samcofer samcofer commented Jun 9, 2023

Pull quarto versions dynamically from github API. The results from the API are paginated with 100 results per page and there are a lot of releases.... 1300ish right now. Additionally, I exclude pre-lease versions to make sure we're only installing properly released versions.

@samcofer
Copy link
Contributor Author

@tnederlof This is ready to review. It's MUCH faster because I added parallelization to make the 5 API calls at once, rather than sequentially. I figured, "I'm in Go, why am I doing things without parallelization?"

@samcofer samcofer requested a review from tnederlof July 11, 2023 17:54
Copy link
Contributor

@tnederlof tnederlof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One quick question otherwise LGTM, thanks for doing this!


for _, result := range results {

err := json.NewDecoder(result.res.Body).Decode(&quarto)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for result.res.Body to not exist and we should check for that or is the only case that would happen the error would be non nil in results which already gets checked earlier?

@dpastoor
Copy link
Contributor

I can't remember if we'd ever talked about this but I do have a reference that uses the excellent go GitHub sdk https://github.com/dpastoor/qvm/blob/main/internal/gh/releases.go

In addition one thing you might want to check before fetching too many releases is if they have an API key, if you're making unauthenticated requests you blast through rate limit and things go south. Then you're blocked for a while so might want to consider how that specific scenario might get bubbled specially.

The other technique to consider is to just check if a known release is valid so if they say provide a given version you can see if it is valid and go get that rather than fetching a bunch.

Great to see this continuing to evolve!

@samcofer samcofer merged commit 306e85d into main May 21, 2024
1 check passed
@samcofer samcofer deleted the dynamic-quarto branch May 21, 2024 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants