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

fix(resource): also ignore no such element err #194

Merged

Conversation

LCaparelli
Copy link
Contributor

@LCaparelli LCaparelli commented Nov 14, 2024

Description of your changes

Since crossplane-runtime might also return a "no such element" error, we must also interpret that case, and ignore it in order to patch the existing array with a new element.

https://github.com/crossplane/crossplane-runtime/blob/19d95a69cc03690c4b867ff91d89681fcf872a93/pkg/fieldpath/paved.go#L124-L126

Previous to this change, we only dealt with the "no such field" error:

https://github.com/crossplane/crossplane-runtime/blob/19d95a69cc03690c4b867ff91d89681fcf872a93/pkg/fieldpath/paved.go#L135-L137

In this case, the lib provides an exported method to check for not found without the need for us to compare against the error's string. This way we also nicely decouple our logic from implementation details on crossplane-runtime.

Fixes #193

I have:

@LCaparelli LCaparelli force-pushed the fix-err-handling-not-found-patch branch 2 times, most recently from 3f8d97f to 393a18c Compare November 14, 2024 22:40
@LCaparelli LCaparelli force-pushed the fix-err-handling-not-found-patch branch 2 times, most recently from 442b3c7 to c530f4f Compare November 14, 2024 23:31
@LCaparelli LCaparelli force-pushed the fix-err-handling-not-found-patch branch from c530f4f to 8b223e7 Compare November 14, 2024 23:32
@Peefy
Copy link
Collaborator

Peefy commented Nov 15, 2024

Thank you. LGTM.

@Peefy Peefy merged commit 1efa293 into crossplane-contrib:main Nov 15, 2024
7 checks passed
@LCaparelli
Copy link
Contributor Author

Thank you @Peefy, that was quick!

When could we expect a release of this fix? I was hoping to get it deployed for something I'm working on this week. We can adapt our logic to avoid the bug, or use a fork, but just wondering if it makes sense to wait for the release. Thanks again!

@LCaparelli LCaparelli deleted the fix-err-handling-not-found-patch branch November 18, 2024 13:56
@Peefy
Copy link
Collaborator

Peefy commented Nov 18, 2024

I've released a new version, and you can test on it. 😊

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

Successfully merging this pull request may close these issues.

Patch targets can't add elements to array
2 participants