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(ui) Retrieve last ingested timestamp and display on frontend #5600

Conversation

chriscollins3456
Copy link
Collaborator

@chriscollins3456 chriscollins3456 commented Aug 9, 2022

This PR gets the timestamp for when a dataset entity was last ingested by checking all of its aspects for the most recent lastObserved in its systemMetadata where the runId exists and is not the default (no-run-id-provided) which means that ingestion is what last touched this aspect.

Here's what it's looking like on the frontend:

within 1 week of ingestion, green
image

on hover
image

hovering over the help icon
image

Orange when it's more than a week but less than a month
image

Red when it's more than a month since
image

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

@github-actions
Copy link

github-actions bot commented Aug 9, 2022

Unit Test Results (build & test)

502 tests   502 ✔️  9m 5s ⏱️
116 suites      0 💤
116 files        0

Results for commit 574de9e.

♻️ This comment has been updated with latest results.

@anshbansal anshbansal added the product PR or Issue related to the DataHub UI/UX label Aug 10, 2022
@@ -62,6 +63,9 @@ public Chart apply(@Nonnull final EntityResponse entityResponse) {
result.setUrn(entityResponse.getUrn().toString());
result.setType(EntityType.CHART);
EnvelopedAspectMap aspectMap = entityResponse.getAspects();
Long lastIngested = SystemMetadataUtils.getLastIngested(aspectMap);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

SystemMetadata systemMetadata = aspectMap.get(aspect).getSystemMetadata();
if (systemMetadata.hasRunId() && !systemMetadata.getRunId().equals(DEFAULT_RUN_ID) && systemMetadata.hasLastObserved()) {
Long lastObserved = systemMetadata.getLastObserved();
if (lastIngested == null || lastObserved > lastIngested) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

@@ -39,6 +40,8 @@ public static Container map(final EntityResponse entityResponse) {
final Container result = new Container();
final Urn entityUrn = entityResponse.getUrn();
final EnvelopedAspectMap aspects = entityResponse.getAspects();
Long lastIngested = SystemMetadataUtils.getLastIngested(aspects);
Copy link
Collaborator

Choose a reason for hiding this comment

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

One thing to keep in mind -- There is a possibility that in the future we become smarter about which aspects we fetch here. At that time we'll need to make some decisions...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

totally - always good to keep that in mind!

entityRegistry: EntityRegistry,
entityType: EntityType,
) {
const entityTypeCased =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice refactor!

Copy link
Collaborator

@jjoyce0510 jjoyce0510 left a comment

Choose a reason for hiding this comment

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

Nice work, @chriscollins3456 !

@jjoyce0510 jjoyce0510 merged commit 261163c into datahub-project:master Aug 12, 2022
@xiphl
Copy link
Contributor

xiphl commented Dec 29, 2022

Sorry for reviving this merged PR, @chriscollins3456 , but can I confirm if the logic considers TimeSeriesAspects as well? that is, if i programmatically ONLY form a time series aspect containing datasetStats and include a runId and lastObserved in the MCP, the last synchronized timestamp will also be updated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR or Issue related to the DataHub UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants