Skip to content
This repository has been archived by the owner on Jul 10, 2024. It is now read-only.

SUBMARINE-556. [WEB] Connect workbench with database for metric #349

Closed
wants to merge 4 commits into from

Conversation

kobe860219
Copy link
Contributor

What is this PR for?

Connect workbench with database for metric.

What type of PR is it?

[Feature]

Todos

What is the Jira issue?

https://issues.apache.org/jira/browse/SUBMARINE-556

How should this be tested?

https://travis-ci.org/github/kobe860219/submarine/builds/709419937

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@kobe860219
Copy link
Contributor Author

@pingsutw @liuxunorg Please help me review this pr, thanks a lot.

Copy link
Member

@pingsutw pingsutw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kobe860219
Some comments in the below.

<th>Value</th>
<th>Time</th>
<th>Step</th>
<th>Is_NaN</th>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could remove this line
If Is_NaN is true, we set metric value string to "Nan"

Copy link
Member

@xunliu xunliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kobe860219 You forgot convert the PR title to uppercase. SUBMARINE-556. xxx
Remember next time.

@kobe860219 kobe860219 changed the title Submarine-556. [WEB] Connect workbench with database for metric SUBMARINE-556. [WEB] Connect workbench with database for metric Jul 20, 2020
@kobe860219 kobe860219 requested a review from pingsutw July 20, 2020 07:03
Copy link
Member

@pingsutw pingsutw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kobe860219
LGTM

Copy link
Member

@xunliu xunliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kobe860219 Thank you contrirbution this feature.
Will merge if no more comments.

@asfgit asfgit closed this in 73e40c5 Jul 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants