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 Jobs, CronJobs, and standalone Pods to topology view #5818
Add Jobs, CronJobs, and standalone Pods to topology view #5818
Conversation
@jeff-phillips-18 can we add a pod donut on the details page of the standalone pods please? |
dfc6463
to
4dee2cd
Compare
@bgliwa01 Added the Pod donut to the pod details page: |
I don't think we discussed this during Epic Exploration or Design ... I'm curious, should we NOT show Jobs with Completed status in Topology @jeff-phillips-18 @bgliwa01 @sspeiche @spadgett ? |
@serenamarie125 Jobs get cleaned up based on the |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @jeff-phillips-18!
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
}) => { | ||
return ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can do implicit return
import { ResourceOverviewDetails } from './resource-overview-details'; | ||
import { PodsOverview } from './pods-overview'; | ||
|
||
const JobOverviewDetails: React.SFC<JobOverviewDetailsProps> = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const JobOverviewDetails: React.SFC<JobOverviewDetailsProps> = ({ | |
const JobOverviewDetails: React.FC<JobOverviewDetailsProps> = ({ |
<DetailsItem label="Desired Completions" obj={job} path="spec.completions" /> | ||
<DetailsItem label="Parallelism" obj={job} path="spec.parallelism" /> | ||
<DetailsItem label="Active Deadline Seconds" obj={job} path="spec.activeDeadlineSeconds"> | ||
{job.spec.activeDeadlineSeconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will it be good to have a check ?
{job.spec.activeDeadlineSeconds | |
{job?.spec?.activeDeadlineSeconds |
@@ -0,0 +1,99 @@ | |||
import * as _ from 'lodash-es'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import * as _ from 'lodash-es'; | |
import * as _ from 'lodash'; |
currentPodCount = pods.length; | ||
title = `${currentPodCount}`; | ||
subTitle = podKindString(currentPodCount); | ||
return { | ||
title, | ||
subTitle, | ||
titleComponent: getTitleComponent(), | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can we have it as below or currentPodCount
being used at other place?
currentPodCount = pods.length; | |
title = `${currentPodCount}`; | |
subTitle = podKindString(currentPodCount); | |
return { | |
title, | |
subTitle, | |
titleComponent: getTitleComponent(), | |
}; | |
return { | |
title : pods.length, | |
subTitle: podKindString(currentPodCount), | |
titleComponent: getTitleComponent(), | |
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has to be a string or a zero value will be ignored (there is a check for !title
in the underlying components).
item: OverviewItem<CronJobKind>; | ||
}; | ||
|
||
export type CronJobResourcesTabProps = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: do we need to export it?
</div> | ||
); | ||
|
||
export const CronJobResourcesTab: React.SFC<CronJobResourcesTabProps> = ({ item }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: do we need to export it ?
Verified locally seems to work as mentioned . can see Jobs, CronJobs, and standalone Pods to topology view along with associated Sidebar Looks good cc @christianvogt |
4dee2cd
to
0cc663f
Compare
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bgliwa01, christianvogt, invincibleJai, jeff-phillips-18, serenamarie125 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 |
Fixes:
https://issues.redhat.com/browse/ODC-4045
https://issues.redhat.com/browse/ODC-4047
https://issues.redhat.com/browse/ODC-4048
https://issues.redhat.com/browse/ODC-4051
https://issues.redhat.com/browse/ODC-4052
Description:
As a Developer, I want to be able to see any workload type in the topology/list view so I can better get a view of all of the computing resources being used. This adds Jobs, CronJobs, Pods.
Screen shots / Gifs for design review:
Topology View:
Job Side Panel:
CronJob Side Panel:
Pod Side Panel:
Browser conformance:
cc @openshift/team-devconsole-ux @serenamarie125 @beaumorley @bgliwa01
/kind feature