feat(ml): bringing ml screens up to date w/ the modern ui layout & improving ml lineage #4651

gabe-lyons · 2022-04-12T22:03:43Z

The ml screens still used the old UI. Additionally, they did not support some modern features like glossary terms and domains. There were also some quirks w/ lineage that got ironed out.

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

github-actions · 2022-04-12T23:38:02Z

Unit Test Results (build & test)

  96 files ±0   96 suites ±0 18m 5s ⏱️ - 2m 55s
689 tests ±0 630 ✔️ ±0 59 💤 ±0 0 ❌ ±0

Results for commit 9fc5d92. ± Comparison against base commit 08c34bf.

♻️ This comment has been updated with latest results.

github-actions · 2022-04-12T23:40:54Z

Unit Test Results (metadata ingestion)

      5 files ±0       5 suites ±0 56m 29s ⏱️ - 6m 21s
  425 tests ±0   425 ✔️ ±  0   0 💤 ±  0 0 ❌ ±0
2 050 runs ±0 1 986 ✔️ - 20 64 💤 +20 0 ❌ ±0

Results for commit 85338f6. ± Comparison against base commit ff685b7.

♻️ This comment has been updated with latest results.

jjoyce0510 · 2022-04-13T01:48:23Z

...e/src/main/java/com/linkedin/datahub/graphql/resolvers/mutate/UpdateDescriptionResolver.java

+        throw new AuthorizationException(
+            "Unauthorized to perform this action. Please contact your DataHub administrator.");
+      }
+      DescriptionUtils.validateLabelInput(targetUrn, _entityService);


Validate label input?

Forget why we called it this...

yeah, this is just an existing method. I think validateDescriptionInput would be a better name- I can refactor as a followup.

jjoyce0510 · 2022-04-13T01:49:17Z

...e/src/main/java/com/linkedin/datahub/graphql/resolvers/mutate/UpdateDescriptionResolver.java

+        throw new AuthorizationException(
+            "Unauthorized to perform this action. Please contact your DataHub administrator.");
+      }
+      DescriptionUtils.validateLabelInput(targetUrn, _entityService);


Seems like some duplicated code in these methods

I know its already the case - later we should see if any of it can be refactored out

yes... the pattern to duplicate code already exists in this resolver. we can look into removing it but its tricky bc each needs to interact w/ its own aspect

jjoyce0510 · 2022-04-13T01:53:49Z

datahub-web-react/src/app/entity/EntityPage.tsx

@@ -60,6 +60,11 @@ export const EntityPage = ({ entityType }: Props) => {
        entityType === EntityType.Chart ||
        entityType === EntityType.DataFlow ||
        entityType === EntityType.DataJob ||
+        entityType === EntityType.Mlmodel ||


jjoyce0510 · 2022-04-13T01:58:39Z

datahub-web-react/src/app/entity/shared/tabs/ML/MlPrimaryKeyFeatureTableTab.tsx

+import { useEntityRegistry } from '../../../../useEntityRegistry';
+import { GetMlPrimaryKeyQuery } from '../../../../../graphql/mlPrimaryKey.generated';
+
+export const FeatureTableTab = () => {


Do we need 2 FeatureTableTab components?

Its a bit simpler this way so we can keep things typesafe

jjoyce0510 · 2022-04-13T01:58:53Z

datahub-web-react/src/graphql/fragments.graphql

@@ -361,7 +361,7 @@ fragment nonRecursiveMLFeature on MLFeature {
    featureNamespace
    description
    dataType
-    featureProperties {
+    properties {


jjoyce0510 · 2022-04-13T01:58:59Z

datahub-web-react/src/graphql/fragments.graphql

@@ -401,7 +413,7 @@ fragment nonRecursiveMLPrimaryKey on MLPrimaryKey {
    featureNamespace
    description
    dataType
-    primaryKeyProperties {
+    properties {


jjoyce0510 · 2022-04-13T01:59:06Z

datahub-web-react/src/graphql/fragments.graphql

@@ -442,7 +466,7 @@ fragment nonRecursiveMLFeatureTable on MLFeatureTable {
        ...platformFields
    }
    description
-    featureTableProperties {
+    properties {


jjoyce0510 · 2022-04-13T01:59:14Z

datahub-web-react/src/graphql/fragments.graphql

    deprecation {
        ...deprecationFields
    }
+    properties {


jjoyce0510 · 2022-04-13T02:00:42Z

metadata-models/src/main/resources/entity-registry.yml

@@ -174,4 +174,39 @@ entities:
    keyAspect: dataPlatformInstanceKey
    aspects:
      - status
+  - name: mlModel
+    category: internal


qq: are these internal? internal usually used for things like policies and secrets that the outside world shouldn't generally look at

Oh ill fix that

jjoyce0510

Overall this LGTM

…proving ml lineage (datahub-project#4651) * backend ml changes * updating ml model UI * more work on the UI * ml primary key joining the party * more progress on UI * making progress on lineage * finalizing UI experience * remove irrelevant test * fixing lint * fixups * add tests and fix what the issues they discovered * internal > core

gabe-lyons added 11 commits April 4, 2022 20:43

backend ml changes

bd1d07e

updating ml model UI

90d1dd0

more work on the UI

716a596

ml primary key joining the party

ea66859

more progress on UI

8b478d6

making progress on lineage

072f0a5

finalizing UI experience

441d884

remove irrelevant test

306913a

Merge branch 'master' into gabe--updatingMlModels

eb58045

fixing lint

db44133

fixups

fb4a205

add tests and fix what the issues they discovered

85338f6

jjoyce0510 reviewed Apr 13, 2022

View reviewed changes

datahub-web-react/src/graphql/fragments.graphql

deprecation {

...deprecationFields

}

properties {

Copy link

Collaborator

jjoyce0510 Apr 13, 2022

Choose a reason for hiding this comment

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

ahhhhh

jjoyce0510 reviewed Apr 13, 2022

View reviewed changes

jjoyce0510 approved these changes Apr 13, 2022

View reviewed changes

internal > core

9fc5d92

gabe-lyons force-pushed the gabe--updatingMlModels branch from feb6e8a to 9fc5d92 Compare April 13, 2022 04:57

gabe-lyons merged commit c92990d into datahub-project:master Apr 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ml): bringing ml screens up to date w/ the modern ui layout & improving ml lineage #4651

feat(ml): bringing ml screens up to date w/ the modern ui layout & improving ml lineage #4651

gabe-lyons commented Apr 12, 2022

github-actions bot commented Apr 12, 2022 •

edited

Loading

github-actions bot commented Apr 12, 2022 •

edited

Loading

jjoyce0510 Apr 13, 2022

jjoyce0510 Apr 13, 2022

gabe-lyons Apr 13, 2022 •

edited

Loading

jjoyce0510 Apr 13, 2022

jjoyce0510 Apr 13, 2022

gabe-lyons Apr 13, 2022

jjoyce0510 Apr 13, 2022

jjoyce0510 Apr 13, 2022

gabe-lyons Apr 13, 2022

jjoyce0510 Apr 13, 2022

jjoyce0510 Apr 13, 2022

jjoyce0510 Apr 13, 2022

jjoyce0510 Apr 13, 2022

jjoyce0510 Apr 13, 2022

gabe-lyons Apr 13, 2022

jjoyce0510 left a comment

feat(ml): bringing ml screens up to date w/ the modern ui layout & improving ml lineage #4651

feat(ml): bringing ml screens up to date w/ the modern ui layout & improving ml lineage #4651

Conversation

gabe-lyons commented Apr 12, 2022

Checklist

github-actions bot commented Apr 12, 2022 • edited Loading

Unit Test Results (build & test)

github-actions bot commented Apr 12, 2022 • edited Loading

Unit Test Results (metadata ingestion)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabe-lyons Apr 13, 2022 • 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

Choose a reason for hiding this comment

jjoyce0510 left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 12, 2022 •

edited

Loading

github-actions bot commented Apr 12, 2022 •

edited

Loading

gabe-lyons Apr 13, 2022 •

edited

Loading