Skip to content

Clean-up Matcher / ResourceUpdatePreProcessor duality #1914

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

Closed
metacosm opened this issue May 24, 2023 · 2 comments
Closed

Clean-up Matcher / ResourceUpdatePreProcessor duality #1914

metacosm opened this issue May 24, 2023 · 2 comments
Assignees

Comments

@metacosm
Copy link
Collaborator

Both concepts are actually quite tied together because usually, if you cannot use the generic matching behavior, this means that you also cannot update the associated resources. This usually means that you need to implement ResourceUpdatePreProcessor if you implement Matcher, for example, if your dependent needs to update the metadata, it probably also need to check whether the actual resource matches the desired one by examining the metadata. However, if you only implement Matcher, your dependent will use the default updating logic, which will only consider the spec part. To fix that problem, you must also implement ResourceUpdatePreProcessor. However, this makes things awkward because we now have a matches method on that interface. We therefore need to unify both Matcher and Updater interfaces by recognising that both are linked and specific behavior in one usually requires specific behavior in the other. This work was started in #1786 but needs to be cleaned-up further.

@metacosm metacosm self-assigned this May 24, 2023
metacosm added a commit that referenced this issue May 24, 2023
Note that the current implementation is flawed because custom
ResourceUpdatePreProcessor should be registered to be found when
attempting to use custom matching logic. See also #1914 for more
details.
metacosm added a commit that referenced this issue May 24, 2023
* fix: clean-up unneeded matcher field

* fix: restore backwards compatibility

Note that the current implementation is flawed because custom
ResourceUpdatePreProcessor should be registered to be found when
attempting to use custom matching logic. See also #1914 for more
details.
@csviri
Copy link
Collaborator

csviri commented May 29, 2023

this is related to this issue:
#1888

@csviri csviri assigned csviri and unassigned metacosm May 30, 2023
@csviri
Copy link
Collaborator

csviri commented May 31, 2023

also related: #1907

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

No branches or pull requests

2 participants