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 broker and trigger datamodel and visualization #5838

Merged

Conversation

karthikjeeyar
Copy link
Contributor

@karthikjeeyar karthikjeeyar commented Jun 26, 2020

Fixes: https://issues.redhat.com/browse/ODC-4161

Analysis / Root cause:
Topology view doesn't support eventing brokers, triggers and its association with knative service

Solution:

  1. Add data model to support broker and trigger
  2. On broker visualization filter out the underlying base resources (InMemoryChannel) from the topology view
  3. Add common actions in the sidebar and resource sections
  4. Supports drag and drop from one service to another
  5. Add create connector via the topology plugin
  6. Add actions to channel/broker on context menu
  7. Add kebab actions for Adding Trigger/subscription.
  8. Hidden internal resources in the topology and showing it in sidebar.

Note: (Things are not covered part of this PR)

  1. Add Trigger/ Add subscription actions will take the user to Add page in this PR (Opening Trigger modal will be handled in next PR)
  2. Sidebar content rearrangement and enhancements will be on next PR.

Screenshots
kebab actions:
image

Broker context menu:
image

Trigger sidebar:
image

GIF:
broker_trigger

Unit Test:

image

image

cc: @serenamarie125 @beaumorley @openshift/team-devconsole-ux @christianvogt

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. component/knative Related to knative-plugin labels Jun 26, 2020
@openshift-ci-robot openshift-ci-robot added the component/dev-console Related to dev-console label Jun 30, 2020
@karthikjeeyar karthikjeeyar force-pushed the feat-broker-trigger branch 2 times, most recently from 73ae21b to 205e492 Compare July 2, 2020 11:21
@karthikjeeyar karthikjeeyar changed the title WIP: Add broker and trigger datamodel and visualization Add broker and trigger datamodel and visualization Jul 2, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 2, 2020
@karthikjeeyar karthikjeeyar force-pushed the feat-broker-trigger branch 4 times, most recently from cfb473f to ffc1732 Compare July 3, 2020 07:04
@openshift-ci-robot openshift-ci-robot added the component/shared Related to console-shared label Jul 3, 2020
@karthikjeeyar karthikjeeyar force-pushed the feat-broker-trigger branch 3 times, most recently from 80da05e to 899f415 Compare July 3, 2020 12:58
@invincibleJai
Copy link
Member

@karthikjeeyar Thanks Verified locally , seems to work as expected with broker / trigger with and without filter's

image

  • perpendicular line for trigger with filter was bit closer as per UXD. See if we can improve.
  • on D&D of eventSourceConnector from a channel to broker, it says Move to Channel this should be as well dynamic i.e depending on channel or broker

@invincibleJai
Copy link
Member

/lgtm

/assign @christianvogt

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 3, 2020
@christianvogt
Copy link
Contributor

The event trigger marker is with a filter is incorrectly drawn as the filter indicator is way too far away from the circle. It moves depending on how far the two nodes are apart.

image

@christianvogt
Copy link
Contributor

Channels and brokers aren't highlighting when moving a connector:
image

@openshift-ci-robot openshift-ci-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 7, 2020
@karthikjeeyar
Copy link
Contributor Author

@christianvogt @invincibleJai I have made the following changes

  1. EventPubSubNode shape updated
  2. Highlighting channels/brokers while moving the connector
  3. Improved the perpendicular line for filter visualization, but still calculates the trigger line position based on the length of connector. (can we fix in later PR ?)

changes

@karthikjeeyar
Copy link
Contributor Author

/retest

@christianvogt
Copy link
Contributor

@karthikjeeyar please create a new issue to address the trigger filter marker

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 8, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, invincibleJai, karthikjeeyar

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-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2020
@openshift-merge-robot openshift-merge-robot merged commit 257547c into openshift:master Jul 8, 2020
@spadgett spadgett added this to the v4.6 milestone Jul 9, 2020
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. component/dev-console Related to dev-console component/knative Related to knative-plugin component/shared Related to console-shared lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants