Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DG-1924 | Add 'assetsCountToPropagate' and 'assetsCountPropagated' in task vertex. #4032
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?
DG-1924 | Add 'assetsCountToPropagate' and 'assetsCountPropagated' in task vertex. #4032
Changes from 10 commits
759f717
65d920d
61d58dd
dfddb79
b4ce7af
ed847c1
c257421
adc6ed1
fdca703
164ec29
4e8c6df
d046141
288062c
ae9dec1
a13f17e
d364c8e
b9b6887
317322e
7cf3072
ba5d144
14db31d
102ae40
64775b4
42540a0
97bbbaf
42b074a
c7c8c4e
6eaf1eb
5caa581
6d176e7
6451bfd
e51846d
48e07f4
547d561
616dcef
06fb81c
cab6deb
157e2db
06e406a
3936029
de81019
720cda0
1b4dbee
e66c636
0f6af6d
73c7cd8
edebe8c
6961d65
5f31f32
8063ff0
b0aadc1
5d31524
08fa364
77dd858
817418b
613db6c
18ccd0f
d7f5e47
4e6eb48
0d8bdf9
386b92c
850fffe
65cb0fb
6aede5c
22e973a
2991b78
75a8e14
bc27ce9
0b8dcd0
754ceb0
e297cae
c1de532
c1fa864
6abecbb
a4b9046
050c1f3
c35fde8
fda9378
7c89b28
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 (propagatedCount == CHUNK_SIZE){
Why this check?
propagatedCount is incremented per classification, not per classification<>asset propagation
I am not sure how does this works but IMO TASK_ASSET_COUNT_PROPAGATED will never get updated if classification count is less than CHUNK_SIZE
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.
for a
classification relationship update
to happenaddTagPropagation
andremoveTagPropagation
both are performed. The number in total could be in hundreds, i am just keeping that check to updatedassetsCountPropagated
every 100 actions be it addition or removalThere 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.
I suspect whether testing this scenario is done with a large dataset, can you ensure the asset on end2 of the relationship that you are going to update has further columns & validate this again for final count as well as incremental count updates
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 was fairly tested with a dataset that has columns on both end of the relationship and the output was correct.
The total lineage count is 5812 for this dataset and one tag was propagated both for hierarchy and lineage. Changing the "propagateTags" from "TWO_TO_ONE" to "NONE" resulted in removal of one tag.
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.
You mentioned that "The total lineage count is 5812", with this can you please justify how the tested value (
1
) is correct & why we should not expect 5812 as value forassetsCountToPropagate
?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.
let's talk in office hours