feat(graph) Add createdOn, createdActor, updatedOn, updatedActor to graph edges #6615

chriscollins3456 · 2022-12-02T17:49:24Z

This PR adds the mechanism to define what the createdOn and updatedOn timestamps are on a graph index edge as well as the createdActor and updatedActor. In order to specify, all you need to do is set the path to the field on the aspect inside of the Relationship annotation. As an example, this is what you can do to set these fields on Upstream in UpstreamLineage

  /**
   * The upstream dataset the lineage points to
   */
  @Relationship = {
    "name": "DownstreamOf",
    "entityTypes": [ "dataset" ],
    "isLineage": true,
    "createdOn": "upstreams/*/created/time"
    "createdActor": "upstreams/*/created/actor"
    "updatedOn": "upstreams/*/auditStamp/time"
    "updatedActor": "upstreams/*/auditStamp/actor"
  }
  @Searchable = {
    "fieldName": "upstreams",
    "fieldType": "URN",
    "queryByDefault": false
  }
  dataset: DatasetUrn

This also sets a default createdOn and createdActor on all new graph edges being created using the metadataChangeLog.systemMetadata.lastObserved timestamp and the metadataChangeLog.created.actor for those fields.

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

…raph index edges

github-actions · 2022-12-02T18:04:48Z

Unit Test Results (build & test)

622 tests +1 618 ✔️ +1 16m 13s ⏱️ -9s
158 suites +1     4 💤 ±0
158 files +1     0 ❌ ±0

Results for commit 1e8c13a. ± Comparison against base commit 4f7b5ac.

♻️ This comment has been updated with latest results.

aditya-radhakrishnan

LGTM! Just some small comments

aditya-radhakrishnan · 2022-12-02T18:03:25Z

metadata-jobs/mae-consumer/src/main/java/com/linkedin/metadata/kafka/hook/GraphIndexUtils.java

+  @Nullable
+  private static List<Urn> getActorList(@Nullable final String path, @Nonnull final RecordTemplate aspect) {
+    List<Urn> actorList = null;
+    if (path != null) {


I actually think this would read cleaner without excessive nesting. So something like

if (path == null) {return null};

if (!value.isPresent() {return null;)

Because then it becomes a bit more obvious in the cases this returns null. Same for the below functions

for sure makes sense!

aditya-radhakrishnan · 2022-12-02T18:04:50Z

...data-jobs/mae-consumer/src/main/java/com/linkedin/metadata/kafka/hook/UpdateIndicesHook.java

@@ -231,7 +231,8 @@ private void updateFineGrainedEdgesAndRelationships(
        // for every downstream, create an edge with each of the upstreams
        for (Urn downstream : fineGrainedLineage.getDownstreams()) {
          for (Urn upstream : fineGrainedLineage.getUpstreams()) {
-            edgesToAdd.add(new Edge(downstream, upstream, DOWNSTREAM_OF));
+            // TODO: add edges uniformly across aspects


What does this todo mean exactly?

yeah it's just that we're adding edges differently for InputFields and fineGrainedLineages right now (like this method and the one below it) and once we add some changes to specifying what the source is, we should be able to remove these methods and just create edges properly along with all the other edges

aditya-radhakrishnan · 2022-12-02T18:20:01Z

...ta-jobs/mae-consumer/src/test/java/com/linkedin/metadata/kafka/hook/GraphIndexUtilsTest.java

+        FieldExtractor.extractFields(upstreamLineage, aspectSpec.getRelationshipFieldSpecs());
+
+    for (Map.Entry<RelationshipFieldSpec, List<Object>> entry : extractedFields.entrySet()) {
+      if (entry.getKey().getPath().toString().equals("/upstreams/*/dataset")) {


where is this path coming from? is this a constant somewhere?

yeah it's just the path I deduced from this specific UpstreamLineage aspect - not a constant anywhere (however I can make it a constant in this test file for readability)

aditya-radhakrishnan · 2022-12-02T18:21:00Z

...ta-jobs/mae-consumer/src/test/java/com/linkedin/metadata/kafka/hook/GraphIndexUtilsTest.java

+  }
+
+  @Test
+  public void testExtractGraphEdgesDefault() {


Do you think there are any other test cases that should be added besides for the default?

I certainly will add more once I start adding changes to relationship annotations on aspects (and add tests for those aspects). I could write one where there's an invalid urn to make sure we don't add that edge.. otherwise other than testing different aspects and checking the types of edges we get are correct I'm not sure.

Okay no worries!

jjoyce0510 · 2022-12-03T01:56:08Z

metadata-jobs/mae-consumer/src/main/java/com/linkedin/metadata/kafka/hook/GraphIndexUtils.java

+  }
+
+  @Nullable
+  private static List<Long> getTimestampList(@Nullable final String path, @Nonnull final RecordTemplate aspect) {


Really nice functions. I like how tight you were able to break them up!

jjoyce0510 · 2022-12-03T01:57:56Z

...ta-jobs/mae-consumer/src/test/java/com/linkedin/metadata/kafka/hook/GraphIndexUtilsTest.java

+
+  @BeforeMethod
+  public void setupTest() {
+    _createdActorUrn = UrnUtils.getUrn(CREATED_ACTOR_URN);


Sweet test! No need to refactor, but I think the firstt 5 of these vars could also be static final variables up top

ah yeah I actually tried doing that first but I think creating an urn from a string up there was causing issues and the build was failing (with a pretty unhelpful error message) - i'm sure there's a different way I could be going about it though 🤷

jjoyce0510

No major comments! Nice.

jjoyce0510 · 2022-12-03T01:58:33Z

Aside from build failing :)

…raph edges (datahub-project#6615)

feat(graph) Add createdOn, createdActor, updatedOn, updatedActor to g…

ff2ed4c

…raph index edges

github-actions bot added the product PR or Issue related to the DataHub UI/UX label Dec 2, 2022

aditya-radhakrishnan approved these changes Dec 2, 2022

View reviewed changes

Mary0054 approved these changes Dec 2, 2022

View reviewed changes

jjoyce0510 reviewed Dec 3, 2022

View reviewed changes

jjoyce0510 approved these changes Dec 3, 2022

View reviewed changes

chriscollins3456 added 3 commits December 5, 2022 10:20

fix broken test

a9f1322

other PR comments

7e9f34d

fix broken tests from not creating edge properly

1e8c13a

chriscollins3456 merged commit b6887d2 into datahub-project:master Dec 5, 2022

cccs-Dustin pushed a commit to CybercentreCanada/datahub that referenced this pull request Feb 1, 2023

feat(graph) Add createdOn, createdActor, updatedOn, updatedActor to g…

9fb2145

…raph edges (datahub-project#6615)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(graph) Add createdOn, createdActor, updatedOn, updatedActor to graph edges #6615

feat(graph) Add createdOn, createdActor, updatedOn, updatedActor to graph edges #6615

chriscollins3456 commented Dec 2, 2022

github-actions bot commented Dec 2, 2022 •

edited

Loading

aditya-radhakrishnan left a comment

aditya-radhakrishnan Dec 2, 2022

chriscollins3456 Dec 2, 2022

aditya-radhakrishnan Dec 2, 2022

aditya-radhakrishnan Dec 2, 2022

chriscollins3456 Dec 2, 2022

aditya-radhakrishnan Dec 2, 2022

aditya-radhakrishnan Dec 2, 2022

chriscollins3456 Dec 2, 2022

aditya-radhakrishnan Dec 2, 2022

aditya-radhakrishnan Dec 2, 2022

chriscollins3456 Dec 2, 2022

aditya-radhakrishnan Dec 2, 2022

jjoyce0510 Dec 3, 2022

jjoyce0510 Dec 3, 2022

chriscollins3456 Dec 5, 2022

jjoyce0510 left a comment

jjoyce0510 commented Dec 3, 2022

feat(graph) Add createdOn, createdActor, updatedOn, updatedActor to graph edges #6615

feat(graph) Add createdOn, createdActor, updatedOn, updatedActor to graph edges #6615

Conversation

chriscollins3456 commented Dec 2, 2022

Checklist

github-actions bot commented Dec 2, 2022 • edited Loading

Unit Test Results (build & test)

aditya-radhakrishnan 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

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

jjoyce0510 commented Dec 3, 2022

github-actions bot commented Dec 2, 2022 •

edited

Loading