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

Testing auto-updater #348

Closed
epruesse opened this issue Oct 20, 2018 · 4 comments
Closed

Testing auto-updater #348

epruesse opened this issue Oct 20, 2018 · 4 comments

Comments

@epruesse
Copy link
Member

While testing, the auto-updater will reference this issue.

This was referenced Oct 20, 2018
This was referenced Oct 24, 2018
@epruesse
Copy link
Member Author

Thats super cool @epruesse! A few things to discuss.

  • rate limit the bot

At this time, it's not a bot but a script. It does have a --max-updates option that will terminate it after n updates completed.

  • randomly update recipes vs. update them in (alphabetically) order

Simple to do, but is this necessary? Eventually, upstream updates will trickle in randomly anyway.

What you are seeing right now is me semi-manually working my way through the alphabet for the recipes the tool can deal with so far. I do this to get started and get updates out of the way, and to have some end-to-end real world test cases. Handling the branches and PRs can't be tested well in the "lab" because of Github and CircleCi interaction.

  • automatic merging of PRs vs. human review

I'm not sure I think that's a really good idea. If you would like to have the PR to have more information to make review simpler, do tell. It's a Jinja template and I can add more data easily.

Perhaps at some point down the road, patch-level updates could be auto-merged. For now, I do think that a pair of human eyes remains necessary. There are just too many things that can go wrong. Github is just like a bio database - if you assume something won't happen because it doesn't make any sense, there surely will be an entry (=repo) in which a user did just that.

  • automatic closing of failed PRs and/or pinging recently maintainers

People listed in extra: recipe-maintainers are already pinged. I could in theory also A) ping the upstream author for GH hosted things, and of course B) ping people who worked on the recipe before. Both of those are "unsolicited pings" - not sure how welcome they are.

Perhaps I could add them quoted to the PR description so that they can be pinged by hand?

Another idea would be pinging them only if the recipe fails to build. That's something an actual bot running on a webhook should be doing though. Not much compute needed, but a reaction to something happening on GH.

  • Should this bot also do maintenance jobs? E.g. removing zlib from the run-time dependencies?

Perhaps. CF calls those things "Migrations". Seeing them as "self patching lints", they could be applied before updating the recipe.

@daler
Copy link
Member

daler commented Oct 24, 2018

First, I want to say again how awesome this is!

Chiming in on a couple of points:

RE: order. Eventually in production we might want to randomize. A pathological case might be if bioconductor has a release at the same time as UCSC, in which case the latter might not get bumped for a while.

RE: auto-merging. Agreed on humans eyes/brains needed. Having the labels on them is great for helping sort though.

RE: pinging. I like the idea of a list of quoted pings including people in git blame and upstream authors to avoid unsolicited notifications but still lower the activation energy of asking for help/clarification.

RE: auto-closing failed. Not sure how I feel about this. Once the autobump-broken label is applied it can be closed to de-clutter the open PRs. Doing a round of cleanup means filtering the closed list by autobump-broken, maybe an autobump-closed label would help identify any auto-closed from manually closed.

RE: linting patches. If the recipe is being touched and looked at, this seems like as good a place as any to do relatively easy, automatable patches.

@epruesse
Copy link
Member Author

RE: order. Eventually in production we might want to randomize. A pathological case might be if bioconductor has a release at the same time as UCSC, in which case the latter might not get bumped for a while.

I'm still not convinced, but since it'll just need a couple of lines to create an extra command line option, I'll just add it.

Regarding Bioconductor: The the recipes currently use {{bioc}} indicating the release with each recipe setting the template to 3.7. So they won't auto-update into a new release anyway.

RE: auto-merging. Agreed on humans eyes/brains needed. Having the labels on them is great for helping sort though.

This may still be something for the future. But looking through more recipes than I had before, there are way too many with only pointless tests. Things like command -v toolname, which ensures only a file toolname with +x in the PATH, but doesn't test that the thing will even start, much less work in some reasonable fashion.

RE: pinging. I like the idea of a list of quoted pings including people in git blame and upstream authors to avoid unsolicited notifications but still lower the activation energy of asking for help/clarification.

Ok, will add that.

RE: auto-closing failed. Not sure how I feel about this. Once the autobump-broken label is applied it can be closed to de-clutter the open PRs. Doing a round of cleanup means filtering the closed list by autobump-broken, maybe an autobump-closed label would help identify any auto-closed from manually closed.

Don't worry - this is just temporary. I'm marking PRs with that label instead of closing them because at the moment the updater would just create a new PR.

I've yet to come up with a good way to avoid retrying already rejected updates.

RE: linting patches. If the recipe is being touched and looked at, this seems like as good a place as any to do relatively easy, automatable patches.

Agreed. Do we have things that should be integrated before I continue updating recipes?

@epruesse epruesse mentioned this issue Nov 14, 2018
28 tasks
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

No branches or pull requests

3 participants