[KED-824] Adopt a new JSON format #2

tolomea · 2019-06-26T14:56:00Z

Notice

I acknowledge and agree that, by checking this box and clicking “Submit Pull Request”:
I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.
I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

Motivation and Context

Why was this PR created?

How has this been tested?

What testing strategies have you used?

Checklist

Read the contributing guidelines
Opened this PR as a 'Draft Pull Request' if it is work-in-progress
Updated the documentation to reflect the code changes
Added new entries to the RELEASE.md file
Added tests to cover my changes
Assigned myself to the PR
Added Type label to the PR

idanov

👍

idanov · 2019-07-11T13:22:08Z

package/kedro_viz/server.py

@@ -21,7 +22,7 @@ def root():


 @app.route("/logs/nodes.json")
-def nodes():
+def nodes_old():


Could we change the name to something like:

Suggested change

def nodes_old():

def nodes_deprecated():

idanov · 2019-07-11T13:29:57Z

package/kedro_viz/server.py

+        )
+        all_tags.update(node.tags)
+        for data_set in node.inputs:
+            namespace = data_set.split("@")[0]


Is the term namespace here the correct one? WDYT @LorenaBalanQB ?

idanov · 2019-07-11T13:41:35Z

package/tests/test_server.py

@@ -10,12 +10,9 @@

 from kedro_viz import server

-EXPECTED_PIPELINE_DATA = [
+EXPECTED_PIPELINE_DATA_OLD = [


⬆️ Same as the comment for nodes_old.

idanov · 2019-07-11T13:44:00Z

package/tests/test_server.py

@@ -49,6 +43,150 @@
 ]


+EXPECTED_PIPELINE_DATA = {


Is it possible to come up with a more minimalistic test case in order to be easier to change later?

idanov · 2019-07-11T13:44:34Z

package/tests/test_server.py

@@ -146,9 +293,17 @@ def test_root_endpoint(client):
    assert "Kedro Viz" in response.data.decode()


-def test_nodes_endpoint(client):
+def test_old_nodes_endpoint(client):


921kiyo

LGTM! I left some comments. Could you please update PR context and tick the box?

921kiyo · 2019-07-15T13:38:41Z

package/kedro_viz/server.py

@@ -21,7 +22,7 @@ def root():


 @app.route("/logs/nodes.json")
-def nodes():
+def nodes_deprecated():


Do we want to throw deprecation warning for this in case users are confused between /api/nodes.json and /logs/nodes.json?

users don't use this, the JS does, retaining the old one is purely so the Python and JS doesn't have to merge at the same time

921kiyo · 2019-07-15T15:05:01Z

package/kedro_viz/server.py

+        parts = [n[0].upper() + n[1:] for n in name.split()]
+        return " ".join(parts)
+
+    pipeline = get_project_context("create_pipeline")()


What would happen if the passed pipeline is broken? Would it fail somewhere inside this function?

it's up to the Pipeline constructor to ensure the constructed pipeline is valid

* Automating Release process (#1) * v6.3.6 * Revert "v6.3.6" This reverts commit 5ffe696. * v6.3.5 * testing release workflow-1 * testing release workflow-1 * testing release workflow-1 * Release/testing (#2) * v6.3.6 * Revert "v6.3.6" This reverts commit 5ffe696. * v6.3.5 * testing release workflow-1 * testing release workflow-1 * testing release workflow-1 * testing release workflow-1 * Release/testing (#3) * v6.3.6 * Revert "v6.3.6" This reverts commit 5ffe696. * v6.3.5 * testing release workflow-1 * testing release workflow-1 * testing release workflow-1 * testing release workflow-1 * final draft * update workflow config * Revert "Release/testing (#3)" This reverts commit d54180d. * merging main * reverting changes to main * final change to extract release notes * update contributing guide * modify Release doc * modify release guide and add release branch condition * fix PR comments

Signed-off-by: Sajid Alam <[email protected]>

* isPrettyName set default to false Signed-off-by: Sajid Alam <[email protected]> * debug Signed-off-by: Sajid Alam <[email protected]> * Update config.js Signed-off-by: Sajid Alam <[email protected]> * Update config.js Signed-off-by: Sajid Alam <[email protected]> * Update app.test.js Signed-off-by: Sajid Alam <[email protected]> * Update app.test.js Signed-off-by: Sajid Alam <[email protected]> * replace space with underscore Signed-off-by: Sajid Alam <[email protected]> * fix tests when pretty name is off by default Signed-off-by: Sajid Alam <[email protected]> * fix Signed-off-by: Sajid Alam <[email protected]> * Update index.js Signed-off-by: Sajid Alam <[email protected]> * more fixes Signed-off-by: Sajid Alam <[email protected]> * fix some failed tests that are related to the prettyName is off by default Signed-off-by: Huong Nguyen <huongg1409@gmail> * Add toLowerCase to some of the tests Signed-off-by: Huong Nguyen <huongg1409@gmail> * Update app.test.js Signed-off-by: Sajid Alam <[email protected]> * add cypress custom command to enable pretty name Signed-off-by: Sajid Alam <[email protected]> * Update commands.js Signed-off-by: Sajid Alam <[email protected]> * Update commands.js Signed-off-by: Sajid Alam <[email protected]> * Update global-toolbar.cy.js Signed-off-by: Sajid Alam <[email protected]> * beforeEach for isPrettyName added Signed-off-by: Jitendra Gundaniya <[email protected]> * menu test fix Signed-off-by: Jitendra Gundaniya <[email protected]> * Update experiment-tracking.cy.js Signed-off-by: Sajid Alam <[email protected]> * attempt #2 Signed-off-by: Sajid Alam <[email protected]> * attempt 3 Signed-off-by: Sajid Alam <[email protected]> * Test fix Signed-off-by: Jitendra Gundaniya <[email protected]> * Update experiment-tracking.cy.js Signed-off-by: Sajid Alam <[email protected]> * revert experiment tracking changes Signed-off-by: Sajid Alam <[email protected]> * Update experiment-tracking.cy.js Signed-off-by: Sajid Alam <[email protected]> * removed enablePrettyNames for exp tracking Signed-off-by: Jitendra Gundaniya <[email protected]> * test fix Signed-off-by: Jitendra Gundaniya <[email protected]> * remove redundant enableprettynames from several tests Signed-off-by: Sajid Alam <[email protected]> * revert removal of enableprettynames for several tests Signed-off-by: Sajid Alam <[email protected]> --------- Signed-off-by: Sajid Alam <[email protected]> Signed-off-by: Huong Nguyen <huongg1409@gmail> Signed-off-by: Jitendra Gundaniya <[email protected]> Co-authored-by: Huong Nguyen <huongg1409@gmail> Co-authored-by: Jitendra Gundaniya <[email protected]>

tolomea added the Enhancement label Jun 26, 2019

tolomea requested review from 921kiyo and nakhan98 June 26, 2019 14:56

richardwestenra closed this Jun 27, 2019

richardwestenra reopened this Jun 27, 2019

tolomea added 5 commits June 28, 2019 10:45

new json format

45ba1b3

tags on datasets and at the top level

455c5ff

an example of an unnamed node

09641aa

tags in tests

c42631f

pretty names for datasets and tags

15c4b23

tolomea force-pushed the new_json branch from 440d911 to 15c4b23 Compare June 28, 2019 13:58

tolomea added 3 commits July 8, 2019 10:20

mark parameters in viz api

fce4886

fix tests

27999c8

do it again, this time with feeling

1eb933d

richardwestenra mentioned this pull request Jul 11, 2019

[KED-824][KED-788] Use new JSON API in app, and calculate transitive links on the fly #8

Merged

8 tasks

idanov approved these changes Jul 11, 2019

View reviewed changes

idanov changed the title ~~[KED-824] New JSON format~~ [KED-824] Adopt a new JSON format Jul 11, 2019

review

1925280

tolomea force-pushed the new_json branch from 9d6369f to 1925280 Compare July 12, 2019 16:19

921kiyo approved these changes Jul 15, 2019

View reviewed changes

richardwestenra merged commit 305ca10 into develop Jul 15, 2019

richardwestenra deleted the new_json branch July 15, 2019 15:18

SajidAlamQB added a commit that referenced this pull request Jul 8, 2024

attempt #2

dde465a

Signed-off-by: Sajid Alam <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KED-824] Adopt a new JSON format #2

[KED-824] Adopt a new JSON format #2

tolomea commented Jun 26, 2019 •

edited

Loading

idanov left a comment

idanov Jul 11, 2019

idanov Jul 11, 2019

idanov Jul 11, 2019

idanov Jul 11, 2019

idanov Jul 11, 2019

921kiyo left a comment

921kiyo Jul 15, 2019

tolomea Jul 15, 2019

921kiyo Jul 15, 2019

tolomea Jul 15, 2019

[KED-824] Adopt a new JSON format #2

[KED-824] Adopt a new JSON format #2

Conversation

tolomea commented Jun 26, 2019 • edited Loading

Notice

Motivation and Context

How has this been tested?

Checklist

idanov left a 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

921kiyo left a 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

tolomea commented Jun 26, 2019 •

edited

Loading