-
Notifications
You must be signed in to change notification settings - Fork 133
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
fix: swaps from github API to expanding the assets #907
fix: swaps from github API to expanding the assets #907
Conversation
That failure seems odd, almost looks like it froze for some reason when I checked timestamps. I would restart it, but I don't think I can. |
Looks like checks passed now, just waiting on feedback from maintainers on the change |
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.
Looks good to me! Thanks a lot! Of course I would prefer the API as we never know what the do with the page layout. Maybe we can keep the API code as well, and just have both options? This way, we could later decide to use a rate limiter or maybe convince github of some higher limits without having to implement everything again.
See my comment on bioconda/bioconda-recipes#42176: if we store etags from the API calls, it’d be trivial to avoid the rate limits. |
@johanneskoester I moved the API version and the expanded asset version into subroutines that are invoked. John's point about the etags / last modified is probably the best way to handle the API in the future, but I'm not sure what long-term memory access autobump would have to retain that info. Those modifications could probably exist in a future PR though. Tests appear to be passing again, and I double checked that results were the same with hiphase and pbfusion recipes. |
🤖 I have created a release \*beep\* \*boop\* --- ## [2.4.0](https://www.github.com/bioconda/bioconda-utils/compare/v2.3.4...v2.4.0) (2023-08-24) ### Features * lint missing run exports ([#908](https://www.github.com/bioconda/bioconda-utils/issues/908)) ([84f5c0c](https://www.github.com/bioconda/bioconda-utils/commit/84f5c0c0b4ec4ba4f1756cbe74da50151efa77a3)) ### Bug Fixes * recover --docker-base-image feature ([#906](https://www.github.com/bioconda/bioconda-utils/issues/906)) ([aa08857](https://www.github.com/bioconda/bioconda-utils/commit/aa088572d3721837d08556634961c5c7d86814c1)) * swaps from github API to expanding the assets ([#907](https://www.github.com/bioconda/bioconda-utils/issues/907)) ([2099a40](https://www.github.com/bioconda/bioconda-utils/commit/2099a405f1b888699eb2026146aa0b4f3f070fb0)) * use package name instead of folder for build failure list dag check ([#910](https://www.github.com/bioconda/bioconda-utils/issues/910)) ([11fb14f](https://www.github.com/bioconda/bioconda-utils/commit/11fb14fd1a95c05dca96887d0d9935f6954447e2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…ts (#912) With #907, we resolved an issue where GitHub release files were not being detected via autobump. Recent logs (e.g. https://circleci.com/api/v1.1/project/github/bioconda/bioconda-utils/44398/output/110/0?file=true&allocation-id=64e7a915cc25c03a3d0cf725-0-build%2FABCDEFGH) show this has changed and it is correctly detecting the version update. However, we are generating new errors now that are specifically caused by the PR submission (this was not previously tested, an oversight on my part): ``` 19:09:44 �[32mBIOCONDA INFO�[0m Recipe hiphase: updating from remote bump/hiphase�[0m 19:09:46 �[31mBIOCONDA ERROR�[0m While processing hiphase Traceback (most recent call last): File "/opt/mambaforge/envs/bioconda/lib/python3.8/site-packages/bioconda_utils/aiopipe.py", line 199, in process await filt.apply(item) File "/opt/mambaforge/envs/bioconda/lib/python3.8/site-packages/bioconda_utils/autobump.py", line 1142, in apply author = self.get_github_author(recipe) File "/opt/mambaforge/envs/bioconda/lib/python3.8/site-packages/bioconda_utils/autobump.py", line 1135, in get_github_author return ver['vals']['account'] KeyError: 'vals'�[0m ``` This is caused by a lack of passing the `vals` fields through from the previous PR. Error was replicated locally via: ``` bioconda-utils autobump recipes/ config.yml --package hiphase --create-pr --dry-run ``` With the very minor changes here, `vals` is passed through and we now get a full run without errors via dry-run: ``` 13:35:00 BIOCONDA INFO Updating checksum for hiphase 0.10.2 13:35:00 BIOCONDA INFO Checking out branch bump/hiphase 13:35:06 BIOCONDA INFO Would create PR 'Update hiphase to 0.10.2' 13:35:06 BIOCONDA INFO title: Update hiphase to 0.10.2 13:35:06 BIOCONDA INFO body: <!-- creator: autobump type: bump_version recipe: hiphase orig_version: 0.10.0 orig_build_number: 0 new_version: 0.10.2 new_build_bumber: 0 --> Update [`hiphase`](https://bioconda.github.io/recipes/hiphase/README.html): **0.10.0** → **0.10.2** [![install with bioconda](https://img.shields.io/badge/install%20with-bioconda-brightgreen.svg?style=flat)](http://bioconda.github.io/recipes/hiphase/README.html) [![Conda](https://img.shields.io/conda/dn/bioconda/hiphase.svg)](https://anaconda.org/bioconda/hiphase/files) Info | Link or Description -----|-------------------- Recipe | [`recipes/hiphase`](https://github.com//bioconda/bioconda-recipes/tree/bump/hiphase/recipes/hiphase) (click to view/edit other files) Summary | Small and structural variant phasing tool for PacBio HiFi reads Home | [https://github.com/PacificBiosciences/HiPhase](https://github.com/PacificBiosciences/HiPhase) Releases |[]() Recipe Maintainer(s) | @holtjma, @ctsa Author | `@PacificBiosciences` *** This pull request was automatically generated (see [docs](https://bioconda.github.io/contributor/updating.html)). 13:35:06 BIOCONDA INFO Would modify PR -1 13:35:06 BIOCONDA INFO New labels: ['autobump', 'new version'] 13:35:06 BIOCONDA INFO Created PR -1: Update hiphase to 0.10.2 100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 1/1 [00:11<00:00, 11.63s/it] 13:35:06 BIOCONDA WARNING Finished update 13:35:06 BIOCONDA INFO Unrecognized URL stats: 13:35:06 BIOCONDA INFO 13:35:06 BIOCONDA INFO Recipe status statistics: 13:35:06 BIOCONDA INFO Updated: 1 13:35:06 BIOCONDA INFO SUM: 1 13:35:06 BIOCONDA WARNING Switching back to master ``` Hopefully, this is the last PR related to this issue!
This is basically a patch to fix a new issue (described here: bioconda/bioconda-recipes#42176) that was introduced by myself here: #896.
The primary cause is that the GitHub API is rate limited (which I was not aware of), so autobump basically blows through the limits quickly and then subsequent calls automatically fail. Of course, this means that the tools we were hoping to see get autobumped are not.
This patch basically removes the original patch (which uses a single GitHub API call) and replaces it with an approach that basically "expands" the release webpage to find the assets. See #895 for more details on how the URLs are masked and the expansion pattern.
Couple other notes:
IncludeFragmentParser
that is basically identical to theHrefParser
except it is looking for tags with this pattern:<include-fragment src="http://url/of/interest" ...>
<include-fragment ...>
tagsPinging @dpryan79 and @johanneskoester as you both reviewed the last related PR. Sorry that the previous PR injected a rate limit issue!
EDIT: I forgot, I have tested these on both
hiphase
andpbfusion
recipes, the diff of the results are here and match our current expectations: