-
Notifications
You must be signed in to change notification settings - Fork 9
LIN-160 - Summary Lineage #4641
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
...ava/org/apache/atlas/repository/store/graph/v2/preprocessor/lineage/LineagePreProcessor.java
Outdated
Show resolved
Hide resolved
repository/src/main/java/org/apache/atlas/discovery/EntityLineageService.java
Outdated
Show resolved
Hide resolved
repository/src/main/java/org/apache/atlas/discovery/EntityLineageService.java
Outdated
Show resolved
Hide resolved
repository/src/main/java/org/apache/atlas/discovery/EntityLineageService.java
Show resolved
Hide resolved
AtlasVertex datasetVertex = AtlasGraphUtilsV2.findByGuid(this.graph, guid); | ||
if (direction == AtlasLineageOnDemandInfo.LineageDirection.INPUT || direction == AtlasLineageOnDemandInfo.LineageDirection.BOTH) | ||
traverseEdgesOnDemand(datasetVertex, true, depth, level, new HashSet<>(), atlasLineageOnDemandContext, ret, guid, inputEntitiesTraversed, traversalOrder); | ||
traverseEdgesOnDemand(datasetVertex, true, depth, level, new HashSet<>(), atlasLineageOnDemandContext, ret, guid, inputEntitiesTraversed, traversalOrder, entityValidationResult); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
traverseEdgesOnDemand has 11 parameters now. This is not getting any easy to maintain.
Always make an attempt to make code better than current! cc: @sumandas0 @aarshi0301
...ava/org/apache/atlas/repository/store/graph/v2/preprocessor/lineage/LineagePreProcessor.java
Outdated
Show resolved
Hide resolved
intg/src/main/java/org/apache/atlas/model/lineage/LineageListRequest.java
Show resolved
Hide resolved
@@ -35,6 +36,9 @@ public void setImmediateNeighbours(Boolean immediateNeighbours) { | |||
this.immediateNeighbours = immediateNeighbours; | |||
} | |||
|
|||
|
|||
private String lineageType = LINEAGE_TYPE_DATASET_PROCESS_LINEAGE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would lineageType always be LINEAGE_TYPE_DATASET_PROCESS_LINEAGE, I do not see any code to conditionally set it to CONNECTION_PROCESS_LINEAGE.
If so we do not need this field at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We pass lineageType from payload, if not present in the payload then we assume default value LINEAGE_TYPE_DATASET_PROCESS_LINEAGE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field can be set from the request payload. If not provided, defaults to LINEAGE_TYPE_DATASET_PROCESS_LINEAGE Supported values: "DatasetProcessLineage" (default), "ConnectionProcessLineage"
@@ -90,6 +94,13 @@ public Integer getDepth() { | |||
public void setDepth(Integer depth) { | |||
this.depth = depth; | |||
} | |||
public String getLineageType() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will alway return LINEAGE_TYPE_DATASET_PROCESS_LINEAGE, please review need of the getter & field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated logic to set based on paload.
@@ -22,6 +23,7 @@ public class LineageOnDemandRequest { | |||
private Set<String> attributes; | |||
private Set<String> relationAttributes; | |||
private LineageOnDemandBaseParams defaultParams; | |||
private String lineageType = LINEAGE_TYPE_DATASET_PROCESS_LINEAGE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same concern for lineageType, it will be always LINEAGE_TYPE_DATASET_PROCESS_LINEAGE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done above^
@@ -35,6 +38,7 @@ public AtlasLineageListContext(LineageListRequest lineageListRequest, AtlasTypeR | |||
this.vertexTraversalPredicate = constructInMemoryPredicate(typeRegistry, lineageListRequest.getEntityTraversalFilters()); | |||
this.edgeTraversalPredicate = constructInMemoryPredicate(typeRegistry, lineageListRequest.getRelationshipTraversalFilters()); | |||
this.attributes = lineageListRequest.getAttributes(); | |||
this.lineageType = lineageListRequest.getLineageType(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always set value to LINEAGE_TYPE_DATASET_PROCESS_LINEAGE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated setter to set different value based on payload.
} | ||
|
||
for (String connectionProcessQn : connectionProcessQNs) { | ||
if (!checkIfChildProcessExistForConnectionProcess(connectionProcessQn)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If checkIfChildProcessExistForConnectionProcess returns GUIDs, you can avoid further getEntityVertex by atlasObjectId which does not use vertex cache, where GUID lookup with cache
} | ||
} catch (Exception e) { | ||
try { | ||
String value = vertex.getProperty(PARENT_CONNECTION_PROCESS_QUALIFIED_NAME, String.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, why are you expecting String PARENT_CONNECTION_PROCESS_QUALIFIED_NAME? I think this is not needed
Change description
Connection level lineage
notion doc - https://www.notion.so/atlanhq/Summary-Lineage-eef721c5acfb48f9b5d3b84aa3da0d43
I have added new file LineagePreProcessor - >
Type of change
Related issues
Helm Config Changes for Running Tests (Staging PR)
Does this PR require Helm config changes for testing?
enpla9up36
. (You can proceed with the PR.) ✅Checklists
Development
Security
Code review