feat(gms) Add aspect mappings to graphql fields to improve request efficiency #15274
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
product
datahub-cyborg
bot
added
the
needs-review
gabe-lyons
reviewed
Nov 11, 2025
Comment on lines
1931
to
+1932
| fineGrainedLineages: [FineGrainedLineage!] | ||
| @aspectMapping(aspects: ["upstreamLineage"]) |
Contributor
There was a problem hiding this comment.
@chriscollins3456 do we actually need upstreamLineage here?
Contributor
There was a problem hiding this comment.
i would hope this would come from elastic?
Collaborator
Author
There was a problem hiding this comment.
this is actually needed i'm pretty sure in our lineage UI in order to show CLL. I can double check with @asikowitz though to be sure.
datahub-cyborg
bot
added
pending-submitter-response
Collaborator
|
Seems like a reasonable approach, annotate the graphql schema with the mapping to aspects required. Some manual/AI work to setup, but I think the it makes a lot of sense. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Note: This still needs tests right now
This is a POC PR that adds aspect mappings to GraphQL fields for the Dataset entity. The goal of this PR is to pave a path forward for us to optimize our entity hydration SQL queries by only fetching the aspects that we need based on the GraphQL query, as opposed to fetching all aspects on every hydration of an entity like we do right now.
In order to do this, I added a few new directives to our graphql schema to map the aspects that populate those fields. This should only be necessary on top level fields for a given entity, like I have in
entity.graphqlhere for Datasets.When we go to hydrate a Dataset, check which fields are being queried for in the graphql query, then check the fields on a Dataset, and get only the required aspects for those fields.
If we're ever in a position where someone adds a new field and forgets to add the aspect directive, I added the conservative fallback to fetch all aspects like before.