Skip to content

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)
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