-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
fix(ui): Reconnect to DAG. Fixes #4301 #4378
Conversation
Signed-off-by: Alex Collins <alex_collins@intuit.com>
…details.tsx Signed-off-by: Alex Collins <alex_collins@intuit.com>
@@ -47,6 +47,10 @@ export class WorkflowDetails extends React.Component<RouteComponentProps<any>, W | |||
private changesSubscription: Subscription; | |||
private timelineComponent: WorkflowTimeline; | |||
|
|||
private get resourceVersion() { | |||
return this.state.workflow && this.state.workflow.metadata.resourceVersion; |
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.
If this.state.workflow.metadata === null
, then we could have a NPE
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.
I don't think this.state.workflow.metadata
could ever be null? That would be an anonymous resource. I don't think you can have them?
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.
Even so, it would probably be good for our code to be tolerant
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.
Up to you
Signed-off-by: Alex Collins <alex_collins@intuit.com> Signed-off-by: Alex Capras <alexcapras@gmail.com>
Signed-off-by: Alex Collins alex_collins@intuit.com
Checklist:
"fix(controller): Updates such and such. Fixes #1234"
.I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.