migrate knative connector side panels to use dynamic extensions #10139

debsmita1 · 2021-09-27T18:18:42Z

Task:
https://issues.redhat.com/browse/ODC-6358

Solution description:
Migrated traffic distributor connector , event source link & kafka connection link sidepanels

debsmita1 · 2021-09-27T18:19:51Z

sahil143 · 2021-09-28T10:08:09Z

...ckages/knative-plugin/src/topology/components/side-panel/knative-connectors-tab-sections.tsx

+    case TYPE_REVISION_TRAFFIC:
+      return i18next.t('knative-plugin~Traffic distribution connector');
+    case TYPE_EVENT_SOURCE_LINK:
+      return i18next.t('knative-plugin~Event source connector');
+    case TYPE_KAFKA_CONNECTION_LINK:
+      return i18next.t('knative-plugin~Kafka connector');


I would suggest adding these as the label on edge data model while forming the model data transformers. Then we wouldn't need to create resource link sections for the type.

element.getLabel() is being used incase resource-link is not found in SideBarHeader component

sahil143 · 2021-09-28T10:23:51Z

frontend/packages/topology/src/components/side-bar/TopologyEdgePanel.tsx

@@ -1,3 +1,5 @@
+// remove this file after migrating all the connectors


Please add a depricated comment on the component also

debsmita1 · 2021-09-30T18:35:25Z

frontend/packages/topology/src/actions/provider.ts

@@ -17,3 +26,26 @@ export const useTopologyWorloadActionProvider = (element: GraphElement) => {
    return [actions, true, undefined];
  }, [actions, element]);
 };
+
+export const useTopologyEdgeActionProvider = (element: BaseEdge) => {


Had to add this hook here because importing edgeActions in the knative-plugin providers increases the main vendor bundle size

yeah, but this may not be ideal to have this in topology. will recommend check with @sahil143 once if we can handle it and avoid the bundle issue. cc @sahil143

@invincibleJai @sahil143 The problem is the moveConnection action uses types from the react-topology package

Ideally we shouldn't be using edgeActions now. It's bad util which pulls action conditionally and not just for Event source link or traffic connector but for visual connector and SBR as well.

try moving the code in this hook from edgeAction and get available target only for the edge that this hook is contributing actions.

debsmita1 · 2021-10-06T13:18:14Z

/retest

invincibleJai · 2021-10-08T07:54:23Z

frontend/packages/knative-plugin/src/actions/providers.ts

+      const sourceModel = modelFor(referenceFor(element.getSource().getData().resource));
+      return [moveSinkSource(sourceModel, element.getSource().getData().resource)];


Suggested change

const sourceModel = modelFor(referenceFor(element.getSource().getData().resource));

return [moveSinkSource(sourceModel, element.getSource().getData().resource)];

const resource = element.getSource().getData().resource;

const sourceModel = modelFor(referenceFor(resource));

return [moveSinkSource(sourceModel, resource)];

invincibleJai · 2021-10-08T07:56:58Z

frontend/packages/topology/src/actions/provider.ts

@@ -17,3 +26,26 @@ export const useTopologyWorloadActionProvider = (element: GraphElement) => {
    return [actions, true, undefined];
  }, [actions, element]);
 };
+
+export const useTopologyEdgeActionProvider = (element: BaseEdge) => {


yeah, but this may not be ideal to have this in topology. will recommend check with @sahil143 once if we can handle it and avoid the bundle issue. cc @sahil143

sahil143 · 2021-10-13T12:58:46Z

frontend/packages/topology/src/actions/provider.ts

@@ -17,3 +26,26 @@ export const useTopologyWorloadActionProvider = (element: GraphElement) => {
    return [actions, true, undefined];
  }, [actions, element]);
 };
+
+export const useTopologyEdgeActionProvider = (element: BaseEdge) => {


Ideally we shouldn't be using edgeActions now. It's bad util which pulls action conditionally and not just for Event source link or traffic connector but for visual connector and SBR as well.

try moving the code in this hook from edgeAction and get available target only for the edge that this hook is contributing actions.

sahil143 · 2021-10-20T13:59:07Z

/retest

sahil143

/lgtm
/label qe-approved

verified changes locallly

invincibleJai · 2021-10-25T15:54:18Z

frontend/packages/knative-plugin/src/actions/providers.ts

@@ -193,3 +197,26 @@ export const useUriActionsProvider = (element: GraphElement) => {
    return [actions, true, undefined];
  }, [actions]);
 };
+
+export const useKnativeConnectorActionProvider = (element: Edge) => {


nit: can we return type

invincibleJai · 2021-10-26T08:58:37Z

/approve

openshift-ci · 2021-10-26T08:59:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: debsmita1, invincibleJai, sahil143

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:

~~frontend/OWNERS~~ [invincibleJai]

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

debsmita1 · 2021-10-26T16:21:37Z

/label px-approved
/label docs-approved

openshift-ci bot added the component/dev-console Related to dev-console label Sep 27, 2021

openshift-ci bot requested review from jcaianirh and karthikjeeyar September 27, 2021 18:19

openshift-ci bot assigned invincibleJai Sep 27, 2021

openshift-ci bot requested a review from sahil143 September 27, 2021 18:19

sahil143 suggested changes Sep 28, 2021

View reviewed changes

debsmita1 force-pushed the knative-connectors branch 3 times, most recently from bc8f55d to 0c2d633 Compare September 30, 2021 18:31

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2021

debsmita1 commented Sep 30, 2021

View reviewed changes

debsmita1 force-pushed the knative-connectors branch from 0c2d633 to 2c287ff Compare September 30, 2021 18:38

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2021

debsmita1 force-pushed the knative-connectors branch from 2c287ff to 48be4c4 Compare October 4, 2021 18:21

debsmita1 requested a review from sahil143 October 4, 2021 18:21

debsmita1 force-pushed the knative-connectors branch 2 times, most recently from 7bc1340 to dfd7dc8 Compare October 6, 2021 06:01

invincibleJai reviewed Oct 8, 2021

View reviewed changes

debsmita1 force-pushed the knative-connectors branch 2 times, most recently from 60d8562 to 2ef416e Compare October 8, 2021 10:39

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 10, 2021

debsmita1 force-pushed the knative-connectors branch from 2ef416e to 63178bb Compare October 11, 2021 14:52

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 11, 2021

debsmita1 force-pushed the knative-connectors branch from 63178bb to 5f9b7d2 Compare October 11, 2021 18:17

sahil143 suggested changes Oct 13, 2021

View reviewed changes

debsmita1 changed the title ~~migrate knative connector side panels~~ migrate knative connector side panels to use dynamic extensions Oct 14, 2021

debsmita1 force-pushed the knative-connectors branch 2 times, most recently from 1b6236d to 16536c6 Compare October 14, 2021 18:10

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 17, 2021

abhinandan13jan mentioned this pull request Oct 18, 2021

[ODC-6371] Migrate contextmenu for VmNode, Service Binding #10179

Merged

4 tasks

debsmita1 force-pushed the knative-connectors branch from 16536c6 to 29247cb Compare October 18, 2021 13:12

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 18, 2021

migrate knative connector side panels

81cf442

debsmita1 force-pushed the knative-connectors branch from 29247cb to 81cf442 Compare October 20, 2021 17:39

debsmita1 requested review from sahil143 and invincibleJai October 21, 2021 06:36

sahil143 approved these changes Oct 21, 2021

View reviewed changes

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Oct 21, 2021

openshift-ci bot assigned sahil143 Oct 21, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 21, 2021

invincibleJai reviewed Oct 25, 2021

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2021

openshift-ci bot added px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR labels Oct 26, 2021

openshift-merge-robot merged commit 7a9aad2 into openshift:master Oct 26, 2021

spadgett added this to the v4.10 milestone Dec 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate knative connector side panels to use dynamic extensions #10139

migrate knative connector side panels to use dynamic extensions #10139

debsmita1 commented Sep 27, 2021

debsmita1 commented Sep 27, 2021

sahil143 Sep 28, 2021

sahil143 Sep 28, 2021

debsmita1 Sep 30, 2021

invincibleJai Oct 8, 2021

debsmita1 Oct 11, 2021

sahil143 Oct 13, 2021

debsmita1 commented Oct 6, 2021

invincibleJai Oct 8, 2021

invincibleJai Oct 8, 2021

sahil143 Oct 13, 2021

sahil143 commented Oct 20, 2021

sahil143 left a comment

invincibleJai Oct 25, 2021

invincibleJai commented Oct 26, 2021

openshift-ci bot commented Oct 26, 2021

debsmita1 commented Oct 26, 2021

		@@ -1,3 +1,5 @@
		// remove this file after migrating all the connectors

		const sourceModel = modelFor(referenceFor(element.getSource().getData().resource));
		return [moveSinkSource(sourceModel, element.getSource().getData().resource)];

-      const sourceModel = modelFor(referenceFor(element.getSource().getData().resource));
-      return [moveSinkSource(sourceModel, element.getSource().getData().resource)];
+      const resource = element.getSource().getData().resource;
+      const sourceModel = modelFor(referenceFor(resource));
+      return [moveSinkSource(sourceModel, resource)];

migrate knative connector side panels to use dynamic extensions #10139

migrate knative connector side panels to use dynamic extensions #10139

Conversation

debsmita1 commented Sep 27, 2021

debsmita1 commented Sep 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

debsmita1 commented Oct 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sahil143 commented Oct 20, 2021

sahil143 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

invincibleJai commented Oct 26, 2021

openshift-ci bot commented Oct 26, 2021

debsmita1 commented Oct 26, 2021