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

✨ Add transformations during request forwarding... #2221

Merged
merged 1 commit into from
Oct 27, 2022

Conversation

davidfestal
Copy link
Member

Summary

Add a mechanism that :

  • provides transformations during request forwarding
  • also allows transforming the request result.

This PR is the first part of PROOF PR #1239, which implements most of the EPIC Multi-SyncTarget syncing and coordination controllers.

Thus this PR should be reviewed with the wider context and use-case in mind.

Related issue(s)

#1992

@stevekuznetsov
Copy link
Contributor

Without seeing this used somewhere, it's not entirely clear to me how to evaluate the client.

@davidfestal
Copy link
Member Author

Without seeing this used somewhere, it's not entirely clear to me how to evaluate the client.
Sure I should have given precise pointers in the PR description.

You can see it in action in the PROOF PR:

This is the first use of it, but @jmprusi is already working on using the same transformingClient mechanism, but with a distinct ResourceTransformer implementation, to settle basic transformations in the new upcoming Upsyncer Virtual Workspace (basic transformations are required during UpSyncing for the storage use-case at first, but also later on for Pod upsyncing).

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 26, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 26, 2022
.. this also allows transforming request result

Signed-off-by: David Festal <[email protected]>
@jmprusi
Copy link
Member

jmprusi commented Oct 27, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 27, 2022
@davidfestal
Copy link
Member Author

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 27, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidfestal

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 27, 2022
@davidfestal
Copy link
Member Author

Since it doesn't impact any other place in the code, I approved the PR myself (though I created it) to unblock further work, especially PR #2180.

@davidfestal davidfestal merged commit 24d1b2c into kcp-dev:main Oct 27, 2022
@davidfestal
Copy link
Member Author

cc @sttts

@kcp-ci-bot kcp-ci-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants