Skip to content
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

feat(graph-retriever): implement graph retriever #10241

Merged
merged 9 commits into from
Apr 16, 2024

Conversation

david-leifker
Copy link
Collaborator

@david-leifker david-leifker commented Apr 8, 2024

Custom plugins including validators, read/write mutators, and side effects currently have access to an AspectRetriever to read aspects while performing these functions. This PR introduces a GraphRetriever to also allow fetching related entities from within these functions.

OperationContext Notes:

  • RetrieverContext introduced to contain both an AspectRetriever and a GraphRetriever
  • Purposefully not exposing the OperationContext and its complexities to the validator (and previously mentioned components)
  • OperationContext extended to entity services to be able to inject both retrievers

Spring Notes:

  • As the OperationContext is injecting dependencies, the need for postConstruct methods to address circular dependencies are being eliminated
  • Several Spring factories around AspectRetrievers are eliminated and consolidated in the OperationContextFactory

GraphRetriever Interface:

public interface GraphRetriever {
  int DEFAULT_EDGE_FETCH_LIMIT = 1000;

  /**
   * Access graph edges
   *
   * @param sourceTypes
   * @param sourceEntityFilter
   * @param destinationTypes
   * @param destinationEntityFilter
   * @param relationshipTypes
   * @param relationshipFilter
   * @param sortCriterion
   * @param scrollId
   * @param count
   * @param startTimeMillis
   * @param endTimeMillis
   * @return
   */
  @Nonnull
  RelatedEntitiesScrollResult scrollRelatedEntities(
      @Nullable List<String> sourceTypes,
      @Nonnull Filter sourceEntityFilter,
      @Nullable List<String> destinationTypes,
      @Nonnull Filter destinationEntityFilter,
      @Nonnull List<String> relationshipTypes,
      @Nonnull RelationshipFilter relationshipFilter,
      @Nonnull List<SortCriterion> sortCriterion,
      @Nullable String scrollId,
      int count,
      @Nullable Long startTimeMillis,
      @Nullable Long endTimeMillis);
}

Example Interface Change For Validator (other interfaces are similar)

AspectRetriever -> RetrieverContext (with getters for Aspect & Graph versions)

  public final Stream<AspectValidationException> validatePreCommit(
      @Nonnull Collection<ChangeMCP> changeMCPs, AspectRetriever aspectRetriever) {

  public final Stream<AspectValidationException> validatePreCommit(
      @Nonnull Collection<ChangeMCP> changeMCPs, @Nonnull RetrieverContext retrieverContext) {

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

@Nonnull TimeseriesAspectService timeseriesAspectService,
@Nonnull UsageClientCacheConfig cacheConfig) {
this.timeseriesAspectService = timeseriesAspectService;
this.operationContextMap = Caffeine.newBuilder().maximumSize(500).build();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lets add this as a constant!

* add graph retriever
* extend operation context to entity services
   * reduce postConstruct() workarounds for circular dependencies
* retriever context add (aspect + graph retriever)
@david-leifker david-leifker merged commit 731c29e into datahub-project:master Apr 16, 2024
35 of 36 checks passed
sleeperdeep pushed a commit to sleeperdeep/datahub that referenced this pull request Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops PR or Issue related to DataHub backend & deployment product PR or Issue related to the DataHub UI/UX release-notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants