-
Notifications
You must be signed in to change notification settings - Fork 763
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
Allow for retrieving latest prerelease as data source. #1066
Conversation
@kfcampbell anything I need to do on this to get it into a reviewable state? Haven't done work on the provider before so wanted to make sure I'm not missing anything. Thanks in advance! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a couple of thoughts, and I approved the Action to run. Thanks for creating this!
Ideally we'd have a documentation update to go on the website for the latest_prerelease
functionality as well.
// TODO: 10 is sort of arbitrary here -- what's the best way to allow configurability | ||
// for this to prevent the provider from becoming glacially slow on large repos? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an excellent question. Do you know what the max number of releases we can retrieve per page with the API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100 is the limit.
for _, rel := range releases { | ||
if *rel.Prerelease != true || *rel.Draft == true { | ||
continue | ||
} | ||
if release == nil { | ||
release = rel | ||
} else { | ||
if rel.PublishedAt.After(release.PublishedAt.Time) { | ||
release = rel | ||
} | ||
} | ||
} | ||
if response.NextPage > nextPage { | ||
nextPage = response.NextPage | ||
} else { | ||
break | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little uncomfortable with this logic as-is. What do you think about refactoring it to a function like GetLatestRelease
that we could write a unit test for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I like that. Will take a stab at it this weekend.
Still haven't had a moment to swing back to this, but I did open a post on the GitHub community for something similar: https://github.community/t/release-api-should-include-a-way-to-retrieve-the-latest-prerelease/239836 It'd be more ideal to handle it at the API level, I think, though that's an ask for core GitHub team and I imagine a fairly large one to think through. |
👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the |
👋 Hey Friends, this pull request has been automatically marked as |
Description
Being able to retrieve the latest release from a given repository is awesome, but there's circumstances in which it'd be beneficial to grab the latest prerelease. In a continuous deployment workflow where there's a "prerelease" like environment (staging, preproduction, prerelease, rc, etc.) it'd be great to be able to pull the tag information directly from the provider, rather than having to skip out to the API to do it.
This PR introduces that functionality by allowing for passing
latest_prerelease
as a valid value toretrieve_by
on the release data source. It's a little more involved than retrieving just the latest release, because the GitHub API at the moment doesn't allow for allowing prereleases as a flag on/latest
. Instead, we're retrieving a few pages worth of releases, filtering out the prereleases, and selecting the most recently published one.This does mean that there's a pretty substantial performance difference, as there's both more data to process and more requests being made.