Staged-recipes: Pushing commit to master on total conversion failure

[jakirkham]

It is common enough that the conversion job fails due to some transient issue networking or Web API (e.g. CIs) that we end up having to restart frequently. If at least a few things convert, we try to save our progress by removing those things that converted and retrigger Travis CI to perform conversion. However, we don't do this if conversion fails to convert any recipe. Instead we just fail on master and it is up to someone with permissions to check or some other user to ping the right people.

Given how common it is to have a transient failure on master and the turnaround time on getting it fixed, it seems that it would make sense to just autoretry. Perhaps we can limit it to a certain number of retries (maybe 5). To do this we can have the conversion process autoretrigger itself by pushing a commit (ideally non-empty) so as to retrigger Travis CI. This commit could be writing the current retry count to a file for instance. If conversion ever succeeds (even partially), we can clear the retry count file.

Thoughts on this idea?

cc @conda-forge/core @conda-forge/staged-recipes

[ericdill]

:+1: from me on enabling auto-retry. 3 or 5 retries per build seem like sensible starting points

[patricksnape]

Fine with me +1

[mwcraig]

:+1:

[ericdill]

Can you help me understand something? In these travis-ci logs it appears that there were two recipes that were successfully turned into feedstocks (typogrify and mu-repo), but only typogrify was removed from the master branch after one of the other feedstocks failed. Why did only typogrify get found and deleted from the master branch?

[ericdill]

Oh I see. The feedstocks get created first, then after all feedstocks are created, the CI services are registered. That travis-ci log that I linked above bombed on registering the CI services for the mu-repo, but succeeded for typogrify. So then it removed the typogrify recipe but needed to keep the mu-repo recipe. Sorry for the noise!

[jakirkham]

Yep, that's what I was meaning by partial conversion as mentioned in the OP. It's totally fair to ask about this stuff. Probably should be documented so that it is more generally known/understood.

[jakirkham]

Have enabled a daily cron job on this repo with Travis. I know that isn't exactly what is proposed in this issue. However, we are regularly having recipes sit for a few days. So it seems like this would help and probably not hurt.

[ericdill]

As I thought about this "push a new commit to master" a little bit more it seems that it gets complicated rather quickly to maintain such a process. You could (pretty easily) get in a condition where the infrastructure put in place would be happily retrying the master branch but then a new PR gets merged into master. Then there's the possibility that you could be trying to create the same feedstock at the same time in two different master branch builds on travis-ci (unless that's already prevented by only building the master branch one at a time or something). Even if parallel master branch builds are not a problem, I suppose you'd still have to check "old" builds that are retrying (up to N times) to see if there are new commits in master, right? And then abort if there are? Or I suppose you'd have to just reset the branch to origin/master? Either way things seems to get somewhat complicated.

tl;dr daily cron job is an easy win here. Good call @jakirkham 😀

[jakirkham]

Thanks for the support.

To try to summarize what you are saying @ericdill, it sounds like you are concerned about race cases at staged-recipes, correct? FWIW we already encounter this situations when several merges occur at staged-recipes in short order.

When it comes to trying to create the same feedstock around the same time, conda-smithy is smart enough to gracefully handle this case. Though the feedstock may get some extra commits as part of token encryption. This doesn't seem to have any ill effect for the feedstock. Though it does trigger extra unnecessary builds. FWIW this has basically been the case since the beginning.

As for the other issues, this definitely was very problematic a year ago. A very common case was to have a conversion (or part of one) complete only to fail because it could not push to staged-recipes. IOW it was beaten by another conversion job. One would need to push empty commits just to restart the thing, which was pretty annoying. Another annoying issue is some recipes would be converted to feedstocks, but they would not be cleared if any conversion job failed. This resulted in some manual removals needing to be done.

Early last fall I made some changes to make staged-recipes a bit more robust in these various scenarios. First it grabs the latest changes in master before starting conversion. Second we convert as many feedstocks as we can, skipping over CI registration errors if they occur. Third we pull in the latest master changes. Fourth we push our changes back to master. Now these obviously don't fix CI web API failures nor do they stop race conditions from occurring. However they do make staged-recipes more robust to both and provide a few perks.

That said, the cron job is a good thing. Just noting we can still do more in this direction without worrying too much.

TL;DR staged-recipes should be robust enough to handle repeated restarts.

[ericdill]

Thanks for the illuminating explanation. That answers all of my questions.

[jakirkham]

Not exactly the same as the solution proposed above, but PR ( https://github.com/conda-forge/staged-recipes/pull/4096 ) would address this problem as well.