WIP: New UI #438

Akado2009 · 2019-03-26T03:28:20Z

This PR introduces new UI built with React. You can submit/monitor both NAS and HP jobs as well as templates.

Fixes: #373

Work still in progress, but all the functionality works. There is a lot of code duplication, i am gonna refactor it asap.

Youtube link with the demo: https://www.youtube.com/watch?v=WAK37UW7spo

This change is

ss

ыфвфывфы

This reverts commit 7ca1a3a.

MattSich · 2019-04-15T20:40:01Z

pkg/types.go

@@ -10,7 +10,8 @@ const (
 	VizierServiceNamespaceEnvName = "VIZIER_CORE_NAMESPACE"
 	VizierService                 = "vizier-core"
 	VizierPort                    = "6789"
-	ManagerAddr                   = VizierService + ":" + VizierPort
+	// ManagerAddr                   = "10.111.12.89:6789"


if not using this then it can be removed

MattSich · 2019-04-15T20:47:57Z

pkg/ui/frontend/public/manifest.json

@@ -0,0 +1,15 @@
+{
+  "short_name": "React App",


You can rename these for mobile web use when added to the spring board

MattSich · 2019-04-15T21:16:18Z

pkg/ui/frontend/src/components/HP/Create/HPParameters.jsx

+                </div>                
+            </div>
+    )
+}


may be useful to add type checking on the props to components https://reactjs.org/docs/typechecking-with-proptypes.html

Nice call, doing it right now. After it is merged we might add some validation (e.g. warning for different forms).

MattSich · 2019-04-16T13:54:10Z

pkg/ui/frontend/src/components/NAS/Monitor/JobList.jsx

+
+const mapStateToProps = (state) => {
+    return {
+        filteredJobsList: state[module].filteredJobsList,


Why not just pass this from the parent where the data is fetched? I find a lot of value in using a container-component approach. Here's a good article on it: https://medium.com/@learnreact/container-components-c0e67432e005
It's just much easier to follow the flow of data when data is passed down to children. Someone reading this may have a hard time seeing that in order for this to display anything, somewhere at some point someone needs to fetch the data using fetchHPJobs() and it may not be clear where that happens - or it could even be multiple places.

MattSich · 2019-04-16T13:58:09Z

pkg/ui/frontend/src/components/HP/Create/YAML.jsx

+                    theme="tomorrow"
+                    value={props.yaml}
+                    onChange={onYamlChange}
+                    name="UNIQUE_ID_OF_DIV"


why name this UNIQUE_ID_OF_DIV?

MattSich · 2019-04-16T13:59:51Z

pkg/ui/frontend/src/components/NAS/Monitor/JobList.jsx

+
+const styles = theme => ({
+    running: {
+        color: '#8b8ffb',


These could be added to the theme definition for reuse

MattSich · 2019-04-16T14:04:46Z

pkg/ui/frontend/src/components/NAS/Monitor/StepInfo.jsx

+                <Typography variant={"h6"}>
+                    Architecture
+                </Typography>
+                <div id={id} style={{textAlign: "center !important", margin: '0 auto'}} />


this inline style can be pulled out into the style definition. Also, instead of using !important if there's a way to use multiple stringed class declaration, I'd use that. css in js is relatively new (in alpha actually), so not sure if it's possible so if not then !important is fine.

MattSich · 2019-04-16T14:07:59Z

pkg/ui/frontend/src/reducers/hpCreate.js

+                requestNumber: action.number,
+            }
+        case actions.ADD_METRICS_HP:
+            let metricsName = state.metricsName.slice()


let's stay consistent and use semicolons

Gonna add them everywhere.

MattSich · 2019-04-16T14:09:33Z

pkg/ui/frontend/src/components/App.jsx

+            <Route path="/katib/nas_monitor/:id" component={NASJobInfo} />
+            <Route path="/katib/worker" component={Worker} />
+            <Route path="/katib/collector" component={Collector} />
+            {/* <Route exact path="/" component={GenerateFromYaml} />


can this be removed?

googlebot · 2019-04-23T19:51:16Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

This reverts commit f4cd554.

googlebot · 2019-04-23T20:02:00Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

andreyvelich · 2019-04-23T22:51:15Z

@MattSich @richardsliu @YujiOshima We fixed all commentaries, please check it again. Will update README for the UI later.

andreyvelich · 2019-04-23T22:52:02Z

examples/v1alpha1/nasjob-example-RL.yaml

                restartPolicy: Never
  suggestionSpec:
    suggestionAlgorithm: "nasrl"
-    requestNumber: 3
+    requestNumber: 1
    suggestionParameters:
      - name: "lstm_num_cells"
        value: "64"


Please, remove changes in the yaml files.

k8s-ci-robot · 2019-04-25T23:44:58Z

@Akado2009: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
kubeflow-katib-presubmit	`a3955c3`	link	`/test kubeflow-katib-presubmit`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

codebreach · 2019-07-21T18:09:46Z

Is there a plan to merge this?

Alternatively can the code here be build (and deployed) separately...?

gaocegege · 2019-09-29T04:00:05Z

/close

I think we have merged the new UI in other PRs. Now katib has the react-based UI. Thanks for your work!

@codebreach Sorry for the late reply. Now katib already has the UI based on react.

k8s-ci-robot · 2019-09-29T04:00:07Z

@gaocegege: Closed this PR.

In response to this:

/close

I think we have merged the new UI in other PRs. Now katib has the react-based UI. Thanks for your work!

@codebreach Sorry for the late reply. Now katib already has the UI based on react.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Akado2009 added 30 commits February 24, 2019 13:46

Merge pull request #1 from kubeflow/master

cf15cd4

ss

UI without backend added

b9d3294

filterting added on monitorin

3c96661

backend 40% done

33046ee

ismall fix

99c0fdb

table + additional plot adde

b94c8ee

hp monitor

4590729

everything works

621c2c0

UI FIX FOR PARAMS

034c786

small fix

64a72fc

input/output size

038f590

before fetch

a6abfb6

isdmall fix

4000db0

Merge pull request #3 from kubeflow/master

3851f36

ыфвфывфы

ismall fix

616d52b

FIX FOR DEMO| HOLD| EVERYTHING IS STATIC

7ca1a3a

Revert "FIX FOR DEMO| HOLD| EVERYTHING IS STATIC"

b823431

This reverts commit 7ca1a3a.

ias

9de6717

Monitor fixed

aabb89d

hp readyh

e8ad6ef

almost everything

8d11741

TODO UPDATE

30324cf

moving to containerized version

24b1a5d

fional fverison almost

dd95ae0

ismall fix

4edbb0f

ismall

6fed5e9

ialso everytyhing fixed

c6426dc

almost

30fa065

small fix

7dd1359

finalk fix

5399aab

k8s-ci-robot requested a review from richardsliu April 9, 2019 16:33

andreyvelich mentioned this pull request Apr 9, 2019

[v1alpha2] New UI #443

Closed

MattSich reviewed Apr 15, 2019

View reviewed changes

MattSich reviewed Apr 16, 2019

View reviewed changes

richardsliu mentioned this pull request Apr 16, 2019

StudyJob v1alpha2 API version #370

Closed

15 tasks

Revert "small fixes"

bbd3186

This reverts commit f4cd554.

Akado2009 force-pushed the new_ui branch from 217f892 to bbd3186 Compare April 23, 2019 20:01

Akado2009 added 5 commits April 23, 2019 14:12

Merge remote-tracking branch 'upstream/master' into HEAD

57a7ea0

imerge master

e799d34

save deleted

502169e

prop-types added

936b68a

yarn.lock deleted

db79620

andreyvelich reviewed Apr 23, 2019

View reviewed changes

Akado2009 added 2 commits April 23, 2019 17:08

build added

40d435e

newline added

a3955c3

k8s-ci-robot closed this Sep 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: New UI #438

WIP: New UI #438

Akado2009 commented Mar 26, 2019 •

edited

Loading

MattSich Apr 15, 2019

Akado2009 Apr 23, 2019

MattSich Apr 15, 2019

Akado2009 Apr 23, 2019

MattSich Apr 15, 2019

Akado2009 Apr 23, 2019 •

edited

Loading

MattSich Apr 16, 2019

MattSich Apr 16, 2019

Akado2009 Apr 23, 2019

MattSich Apr 16, 2019

Akado2009 Apr 23, 2019

MattSich Apr 16, 2019

Akado2009 Apr 23, 2019

MattSich Apr 16, 2019

Akado2009 Apr 23, 2019

MattSich Apr 16, 2019

Akado2009 Apr 23, 2019

googlebot commented Apr 23, 2019

googlebot commented Apr 23, 2019

andreyvelich commented Apr 23, 2019

andreyvelich Apr 23, 2019

k8s-ci-robot commented Apr 25, 2019

codebreach commented Jul 21, 2019

gaocegege commented Sep 29, 2019

k8s-ci-robot commented Sep 29, 2019

WIP: New UI #438

WIP: New UI #438

Conversation

Akado2009 commented Mar 26, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Akado2009 Apr 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

googlebot commented Apr 23, 2019

googlebot commented Apr 23, 2019

andreyvelich commented Apr 23, 2019

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 25, 2019

codebreach commented Jul 21, 2019

gaocegege commented Sep 29, 2019

k8s-ci-robot commented Sep 29, 2019

Akado2009 commented Mar 26, 2019 •

edited

Loading

Akado2009 Apr 23, 2019 •

edited

Loading