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

use GitHub releases tarballs #108

Closed
wants to merge 33 commits into from

Conversation

ericLemanissier
Copy link
Contributor

@ericLemanissier ericLemanissier commented Mar 28, 2023

fixes #106

This is based on top of #107 to ease my tests, but the change in ccb/upstream_project.py is totally independant from github workfows modifications.

Contrary to other Upstream sub-classes, I had to do the request in GithubReleaseProject constructor (as opposed to the versions(self) property), because we have to know if there are some actual releases during construction, in order to try other Upstream types if there are no actual releases. This also implies that if the token becomes rate limited, the exception is caught, and other Upstream types are tried.
Also, releases tarball download URL cannot be derived from version alone, so I cached the download url as member of the Version object. I could not cache it in the GithubReleaseProject instance because several instances of the detected Upstream are created for each recipe during a single workflow.

Examples of branches created by this modification:

@ericLemanissier ericLemanissier changed the base branch from master to dev March 28, 2023 16:01
mostly blacklist and whitelist for tags
@ericLemanissier ericLemanissier force-pushed the github-releases branch 2 times, most recently from e87452d to 484f10e Compare March 28, 2023 19:32
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@@ -117,15 +117,15 @@ jobs:
--no-link-pr \
--github-token "$GITHUB_TOKEN" \
./status/v1-update.json \
https://github.com/qchateau/conan-center-bot/issues/4
https://github.com/${{ github.repository }}/issues/4
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't you put the issue number in a CI env var ?

Copy link
Owner

Choose a reason for hiding this comment

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

Re-opening this thread as you're working in your fork - it will probably fail if the number is not configurable, right ?
Same line 90

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll put these issues numbers in a repo variable

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/update_status.yml Outdated Show resolved Hide resolved
ccb/upstream_project.py Outdated Show resolved Hide resolved
it allows the use of aiohttp
qchateau
qchateau previously approved these changes Apr 2, 2023
@qchateau qchateau self-requested a review April 2, 2023 13:50
@qchateau
Copy link
Owner

qchateau commented Apr 2, 2023

I wonder why the CI doesn't run on this PR

@ericLemanissier ericLemanissier force-pushed the github-releases branch 3 times, most recently from dadcadd to 545406e Compare May 15, 2023 07:47
@ericLemanissier
Copy link
Contributor Author

ok, I'm finally starting to get some results on my fork : https://ericlemanissier.github.io/conan-center-bot/

Copy link
Owner

@qchateau qchateau left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. I left a few comments to keep track of your work.

@@ -117,15 +117,15 @@ jobs:
--no-link-pr \
--github-token "$GITHUB_TOKEN" \
./status/v1-update.json \
https://github.com/qchateau/conan-center-bot/issues/4
https://github.com/${{ github.repository }}/issues/4
Copy link
Owner

Choose a reason for hiding this comment

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

Re-opening this thread as you're working in your fork - it will probably fail if the number is not configurable, right ?
Same line 90

ccb/upstream_project.py Show resolved Hide resolved
github_token = get_github_token()
headers = {"Accept": "application/vnd.github.v3+json"}
if github_token:
headers["Authorization"] = f"token {github_token}"
Copy link
Owner

Choose a reason for hiding this comment

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

I think the CI now fails due to the rate limit of the token (or am I misinterpreting the logs ?)
I'm especially worried as the number of projects can only grow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it is because I always push 3 branches at the same time: master, dev, and the one of this PR. With each branch triggering 2 workflows, plus the PR's workflow, it makes a lot more requests than actually needed. The cron workflow always passes on my fork.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or maybe they are actually not using the token?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ericLemanissier ericLemanissier force-pushed the github-releases branch 7 times, most recently from 54932de to 78dc856 Compare May 22, 2023 12:41
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.

Use Releases instead of Tags
2 participants