Skip to content

Conversation

@ybz1013
Copy link
Contributor

@ybz1013 ybz1013 commented Jul 16, 2025

Summary

The previous IngestionAspectETag was not uniform with the response type of AspectMetadata. Now, we are introducing AspectMetadata to wrap around metadata of aspects for both request and response payload.

Logic change needed in EbeanLocalDAO when extracting the etag.

Testing Done

./gradlw build
unit tests

Checklist

@ybz1013 ybz1013 requested a review from Copilot July 16, 2025 22:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the ingestion parameter model to use a unified AspectMetadata wrapper for both request and response payloads, replacing the older IngestionAspectETag constructs.

  • Replace ingestionETags array with a request_metadata field of type RequestMetadata in IngestionParams
  • Update EbeanLocalDAO logic and corresponding tests to extract optimistic locks from AspectMetadata instead of IngestionAspectETag
  • Remove the old IngestionAspectETag PDL schema and add new PDL records for AspectMetadata, RequestMetadata, and ResponseMetadata

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
dao-api/src/main/pegasus/com/linkedin/metadata/internal/IngestionParams.pdl Replace ingestionETags with request_metadata: RequestMetadata
dao-api/src/main/pegasus/pegasus/com/linkedin/metadata/events/IngestionAspectETag.pdl Remove deprecated IngestionAspectETag schema
dao-api/src/main/pegasus/pegasus/com/linkedin/metadata/aspect/AspectMetadata.pdl Add new AspectMetadata schema
dao-api/src/main/pegasus/pegasus/com/linkedin/metadata/aspect/RequestMetadata.pdl Add new RequestMetadata schema
dao-api/src/main/pegasus/pegasus/com/linkedin/metadata/aspect/ResponseMetadata.pdl Add new ResponseMetadata schema
dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java Refactor lock-extraction to iterate over RequestMetadata
dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/EbeanLocalDAOTest.java Update tests to build and use RequestMetadata + AspectMetadata
Comments suppressed due to low confidence (3)

dao-api/src/main/pegasus/com/linkedin/metadata/internal/IngestionParams.pdl:30

  • [nitpick] Field 'request_metadata' mixes snake_case with other camelCase fields (e.g. testMode, trackingContext) in IngestionParams. Consider renaming to 'requestMetadata' for consistency.
  request_metadata: optional RequestMetadata

dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java:622

  • Consider adding a unit test for the case where request_metadata is null or has an empty aspect_metadata array to verify that the method safely skips without errors.
    final RequestMetadata requestMetadata = ingestionParams.getRequest_metadata();

dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java:625

  • [nitpick] Consider adding a test with multiple AspectMetadata entries in request_metadata to ensure the method correctly selects the matching alias among several items.
      for (AspectMetadata aspectMetadata: requestMetadata.getAspect_metadata()) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant