Skip to content
This repository has been archived by the owner on Jul 15, 2024. It is now read-only.

fixes RepoHasPath error handling #423

Merged
merged 1 commit into from
Dec 1, 2021

Conversation

benoitg31
Copy link
Contributor

@benoitg31 benoitg31 commented Nov 26, 2021

In the case that gitlab is unreachable , RepoHasPath would return false with NO error , thus filtering out the app from the "desired apps". This is not a wanted behaviour as the check could not be performed correctly.
The fixes checks the error from ListTree first, then checks the results.

In trhe case that gitlab is unreachable previous code would return no error , thus filtering out the app from the desired apps.
This is not a wanted behaviour as the check could not be performed correctly
@CLAassistant
Copy link

CLAassistant commented Nov 26, 2021

CLA assistant check
All committers have signed the CLA.

@benoitg31 benoitg31 changed the title fixes error handling fixes RepoHasPath error handling Nov 26, 2021
@yogeek
Copy link
Contributor

yogeek commented Nov 29, 2021

Would be great to have this PR reviewed ! (and even greater if it could make the v0.3.0 release 🚀)
Indeed, this fix could solve the critical issue I explained in a Slack discussion with @jgwest.
I said "critical" because this issue cause some of our applications to be deleted randomly during our gitlab instance termination (and the linked resources are also deleted because we do not set the preserveResourcesOnDeletion).
Thank you 😃

@jgwest jgwest self-requested a review December 1, 2021 16:06
Copy link
Member

@jgwest jgwest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @benoitg31 and @yogeek!

@jgwest jgwest merged commit ce12bd5 into argoproj:master Dec 1, 2021
@jgwest jgwest added this to the v0.3.0 milestone Dec 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants