edgetest force pushes overwrite user commits

[fdosani]

relevant issue: capitalone/edgetest#22

edgetest opened this PR to update the dask[dataframe] version yesterday

capitalone/rubicon-ml#221

I had to manually apply those version updates to the environment files in my repo, which caused the whitesource scan to fail. so, I left the PR open overnight to come back to it today and finish. overnight, edgetest again noticed that dask[dataframe] needed to be updated so it created a branch with the same name as the one I had committed my manual changes to yesterday and forced pushed, overwriting the branch

edgetest should be able to recognize that it already opened the PR to update dask[dataframe] and not force push the same update

I also imagine that if a second library needed to be updated while the PR for a previous one is still open we'd see the same behavior since the branch always seems to be named edgetest-patch

[ryanSoley]

thanks for moving it to the right place!

[fdosani]

Thanks for opening the issue @ryanSoley I'll take a look at the PR action we are using, there should be a flag to not force push. Let me circle around this.

[fdosani]

@ryanSoley I've been looking into this issue today and not sure there is a clean way to do this. for context here is the pull request action and how it does things. There are a couple of options on how to tackle this.

We could enable usage via a flag of the alternate strategy so that any subsequent runs of edgetest will just open a new PR vs force writing an existing one. It isn't great but atleast prevents you losing your work if you do something else on the branch edgetest opens up.

Ideally folks won't commit things to that branch since it is designed only for edgetest, but that might not always be the case.

Just to gather thoughts here:

I also imagine that if a second library needed to be updated while the PR for a previous one is still open we'd see the same behavior since the branch always seems to be named edgetest-patch

It would re run edgetest and since it would be based off dev it would pick up both package changes so it wouldn't be an issue really.

The real issue is if you make non-edgetest changes to edgetest-patch, these would get lost of you re-ran edgetest before merging. The underlying create-pull-request doesn't allow the base and the branch to be the same. See this error:

Error: The 'base' and 'branch' for a pull request must be different branches. Unable to continue.

I think my recommendation would be to leave edgetest-patch alone and if you do need to make changes then they should be on a different branch. I think this scenario came up due to the whitesource issues we were facing. appreciate your thoughts and happy to chat more over chat if this is a real urgent concern.

[ryanSoley]

@fdosani I agree that the problem should probably be fixed elsewhere and not necessarily here in the push mechanics. It wasn't whitesource issues that brought this up - it was the fact that the auto-PRs do not update conda environment YAML files, only the setup files. If edgetest updated all the necessary files, this wouldn't be an issue

[fdosani]

I'll leave this open for now, I want to mull over it a bit more, as to some solution if there is something less hacky.