create a utility and component for traffic connector #4022

sahil143 · 2020-01-21T12:45:47Z

ODC-story: https://issues.redhat.com/browse/ODC-2461

This Pr adds the traffic connector and is based on the mock data. To test this PR locally run below cmds and select the bookinfo namespace

oc new-project bookinfo
oc apply -n bookinfo -f https://raw.githubusercontent.com/Maistra/bookinfo/maistra-1.0/bookinfo.yaml

andrewballantyne · 2020-01-21T16:03:10Z

/kind feature

cc @openshift/team-devconsole-ux

divyanshiGupta · 2020-01-22T13:17:26Z

frontend/packages/dev-console/src/components/topology/componentFactory.ts

So consts for other connectors doesnt have an edge suffix, for consistency either add edge suffix in all connectors or remove it from this one.

divyanshiGupta · 2020-01-22T13:19:55Z

frontend/packages/dev-console/src/components/topology/topology-utils.ts

Why not use the traffic data type here?

divyanshiGupta · 2020-01-22T13:20:58Z

frontend/packages/dev-console/src/components/topology/topology-utils.ts

Can we use types instead of any?

divyanshiGupta · 2020-01-23T10:19:54Z

/lgtm

divyanshiGupta · 2020-01-23T10:20:58Z

For now I think it is okay to use the mock data and later when we have the kiali api refactor the code to use it. cc: @christianvogt

andrewballantyne

Looks good, needs tests. @sahil143 please log a task in JIRA, assign it to yourself, and put it in the next sprint (179). You'll need to write tests next week.

andrewballantyne · 2020-01-24T12:50:18Z

Not sure how to test the connector part yet; will look to follow https://issues.redhat.com/browse/ODC-2505 shortly.

andrewballantyne · 2020-01-24T15:32:03Z

UX note... this looks weird and not like a dashed line. Is this going to be confusing to the user?

cc @openshift/team-devconsole-ux

Dashed lines don't overlap all that well.

andrewballantyne

This looks good to me minus the overlapping connectors. We'll have to make sure we show this to UX and get feedback...

I'm approving this PR because it looks good to me and is a blocker for @divyanshiGupta 's PR.

@sahil143 Please log that Unit Test ticket when JIRA comes back up.

andrewballantyne

Some changes needed. You can log a ticket and address these later; nothing is critical.

A new test, and a use of a constant over a magic string.

andrewballantyne · 2020-01-24T16:03:03Z

frontend/packages/dev-console/src/components/topology/topology-utils.ts

You modified the output here and it didn't fail a test 😕 I think we need to write a test for this condition too.. otherwise we'll not catch a break if this violates the status quo.

Even if this condition is not true it doesn't affect the edges array. it's just that Traffic connector will be not be added to the edges array.

Ah, doh - right it wouldn't fail pre-existing tests. My brain is half functioning today 😆 But we still need to write a test for if it is truthy.

yes I did for falsy

I'm confused by this comment:

yes I did for falsy

This didn't exist during my last test, did it? I'm 99% I didn't see the falsy test lol.

No, there wasn't any test to check the condition. But now I added for both falsy and truthy. 😉

andrewballantyne · 2020-01-24T16:04:32Z

frontend/packages/dev-console/src/components/topology/topology-utils.ts

I believe there is a type for this (I didn't know this when previously reviewing)...

Suggested change

nodes.filter(({ data }) => data.nodeType === 'workload');

nodes.filter(({ data }) => data.nodeType === TYPE_WORKLOAD);

From frontend/packages/dev-console/src/components/topology/const.ts

openshift-ci-robot · 2020-01-26T03:35:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne, divyanshiGupta, karthikjeeyar, sahil143

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~frontend/packages/dev-console/OWNERS~~ [andrewballantyne]

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

openshift-bot · 2020-01-26T04:42:12Z

/retest