action-autotag icon indicating copy to clipboard operation
action-autotag copied to clipboard

Fix: don't fail if the tag already exists

Open abusque opened this issue 2 years ago • 7 comments

Previously, if the tag to create was deemed to already exist, a warning would be logged and the action would not attempt to create it.

However, a recently merged commit [0] changed this behaviour, by marking the action as failed if the tag already exists, instead of simply emitting a warning.

Therefore, to prevent the action from failing when the tag already exists, revert to the previous behaviour and emit a warning instead.

[0] https://github.com/ButlerLogic/action-autotag/commit/8a4f4d8b4fa0aefbbedf904bcb6fc96defb8bf3d

Signed-off-by: Antoine Busque [email protected]

abusque avatar Oct 17 '22 22:10 abusque

@coreybutler can we merge this please?

tpluscode avatar Mar 02 '23 15:03 tpluscode

I never saw the notice for this PR.

The default behavior should not change because many workflows depend on a tag conflict to produce a failure. For example, rollback operations are often triggered with prior action failures.

I'd like to see a configurable flag to provide the desired behavior for those who need an alternative. For example, an option like fail_on_tag_conflict (true by default) could be set to prevent the failure:

- uses: butlerlogic/[email protected]
  env:
    GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"
  with:
    fail_on_tag_conflict: false

coreybutler avatar Mar 02 '23 18:03 coreybutler

The default behavior should not change because many workflows depend on a tag conflict to produce a failure. For example, rollback operations are often triggered with prior action failures.

Unless I'm mistaken, up until PR #26 was merged on October 12 2022, the default behaviour was to not fail on tag conflict, hence my PR to revert to that original behaviour. Workflows for projects I work on depended on not failing if the tags already existed.

Now of course if you say the new default is to fail if it already exists and you want to keep it this way that's fine, I just want to make sure I understand correctly which ones of these is the intended default.

If you want to keep the fail on tag conflict as default, I can look into adding the option to make it configurable like you suggest. I don't have a ton of bandwidth right now though so I may not be able to update the PR right away.

abusque avatar Mar 02 '23 23:03 abusque

I understand the confusion. PR #26 was merged because failures were happening silently. That PR made the failures noisy and introduced the dry-run concept as a way to detect a noisy failure before actually running anything.

The default should fail. The override can prevent this failure. The train of thought is "the autotag action will fail if it cannot actually tag the code".

The configurable option solves the problem without creating any new ones, which makes sense to me. I too am stretched very thin for time, so I'll mark this as "help wanted". @abusque - if you find yourself freeing up anytime soon and want to keep going, let me know and I'll take the "help wanted" label off.

coreybutler avatar Mar 02 '23 23:03 coreybutler

Understood, thanks for providing the extra context. Adding the extra option shouldn't be too complex so I'll try to find the time to do it, hopefully next week.

abusque avatar Mar 03 '23 14:03 abusque

@Lissy93 I think it's safe to say we're all stretched for bandwidth. A fresh PR implementing this would be very welcome. If you do this, please mention this PR as well so I am sure to close it when the new one is merged. Thank you!

coreybutler avatar Feb 24 '24 17:02 coreybutler

Hi @Lissy93, thanks a lot for the suggested fix. I'm definitely bandwidth constrained, and the projects that prompted me to open this PR have workarounds in place already, so this item has fallen off my priority list. Definitely feel free to open the separate PR that supersedes this one.

abusque avatar Feb 24 '24 23:02 abusque