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

fix(model): fixes DashboardContainsDashboard relationship in DashboardInfo aspect #12433

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.linkedin.datahub.upgrade.config.restoreindices;

import com.linkedin.datahub.upgrade.config.SystemUpdateCondition;
import com.linkedin.datahub.upgrade.system.NonBlockingSystemUpgrade;
import com.linkedin.datahub.upgrade.system.restoreindices.dashboardinfo.ReindexDashboardInfo;
import com.linkedin.metadata.entity.AspectDao;
import com.linkedin.metadata.entity.EntityService;
import io.datahubproject.metadata.context.OperationContext;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Conditional;
import org.springframework.context.annotation.Configuration;

@Configuration
@Conditional(SystemUpdateCondition.NonBlockingSystemUpdateCondition.class)
public class ReindexDashboardInfoConfig {

@Bean
public NonBlockingSystemUpgrade reindexDashboardInfo(
final OperationContext opContext,
final EntityService<?> entityService,
final AspectDao aspectDao,
@Value("${systemUpdate.dashboardInfo.enabled}") final boolean enabled,
@Value("${systemUpdate.dashboardInfo.batchSize}") final Integer batchSize,
@Value("${systemUpdate.dashboardInfo.delayMs}") final Integer delayMs,
@Value("${systemUpdate.dashboardInfo.limit}") final Integer limit) {
return new ReindexDashboardInfo(
opContext, entityService, aspectDao, enabled, batchSize, delayMs, limit);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package com.linkedin.datahub.upgrade.system.restoreindices.dashboardinfo;

import com.google.common.collect.ImmutableList;
import com.linkedin.datahub.upgrade.UpgradeStep;
import com.linkedin.datahub.upgrade.system.NonBlockingSystemUpgrade;
import com.linkedin.metadata.entity.AspectDao;
import com.linkedin.metadata.entity.EntityService;
import io.datahubproject.metadata.context.OperationContext;
import java.util.List;
import javax.annotation.Nonnull;
import lombok.extern.slf4j.Slf4j;

/**
* A job that reindexes all dashboard info aspects as part of reindexing dashboards relationship.
* This is required to fix the dashboards relationships for dashboards
*/
@Slf4j
public class ReindexDashboardInfo implements NonBlockingSystemUpgrade {

private final List<UpgradeStep> _steps;

public ReindexDashboardInfo(
@Nonnull OperationContext opContext,
EntityService<?> entityService,
AspectDao aspectDao,
boolean enabled,
Integer batchSize,
Integer batchDelayMs,
Integer limit) {
if (enabled) {
_steps =
ImmutableList.of(
new ReindexDashboardInfoStep(
opContext, entityService, aspectDao, batchSize, batchDelayMs, limit));
} else {
_steps = ImmutableList.of();

Check warning on line 36 in datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/dashboardinfo/ReindexDashboardInfo.java

View check run for this annotation

Codecov / codecov/patch

datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/dashboardinfo/ReindexDashboardInfo.java#L36

Added line #L36 was not covered by tests
}
}

@Override
public String id() {
return this.getClass().getName();

Check warning on line 42 in datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/dashboardinfo/ReindexDashboardInfo.java

View check run for this annotation

Codecov / codecov/patch

datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/dashboardinfo/ReindexDashboardInfo.java#L42

Added line #L42 was not covered by tests
}

@Override
public List<UpgradeStep> steps() {
return _steps;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.linkedin.datahub.upgrade.system.restoreindices.dashboardinfo;

import static com.linkedin.metadata.Constants.*;

import com.linkedin.datahub.upgrade.system.AbstractMCLStep;
import com.linkedin.metadata.entity.AspectDao;
import com.linkedin.metadata.entity.EntityService;
import io.datahubproject.metadata.context.OperationContext;
import javax.annotation.Nonnull;
import lombok.extern.slf4j.Slf4j;
import org.jetbrains.annotations.Nullable;

@Slf4j
public class ReindexDashboardInfoStep extends AbstractMCLStep {

public ReindexDashboardInfoStep(
OperationContext opContext,
EntityService<?> entityService,
AspectDao aspectDao,
Integer batchSize,
Integer batchDelayMs,
Integer limit) {
super(opContext, entityService, aspectDao, batchSize, batchDelayMs, limit);
}

@Override
public String id() {
return "dashboard-info-v1";

Check warning on line 28 in datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/dashboardinfo/ReindexDashboardInfoStep.java

View check run for this annotation

Codecov / codecov/patch

datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/dashboardinfo/ReindexDashboardInfoStep.java#L28

Added line #L28 was not covered by tests
}

@Nonnull
@Override
protected String getAspectName() {
return DASHBOARD_INFO_ASPECT_NAME;

Check warning on line 34 in datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/dashboardinfo/ReindexDashboardInfoStep.java

View check run for this annotation

Codecov / codecov/patch

datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/dashboardinfo/ReindexDashboardInfoStep.java#L34

Added line #L34 was not covered by tests
}

@Nullable
@Override
protected String getUrnLike() {
return "urn:li:" + DASHBOARD_ENTITY_NAME + ":%";

Check warning on line 40 in datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/dashboardinfo/ReindexDashboardInfoStep.java

View check run for this annotation

Codecov / codecov/patch

datahub-upgrade/src/main/java/com/linkedin/datahub/upgrade/system/restoreindices/dashboardinfo/ReindexDashboardInfoStep.java#L40

Added line #L40 was not covered by tests
}
}
2 changes: 2 additions & 0 deletions docs/how/updating-datahub.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ This file documents any backwards-incompatible changes in DataHub and assists pe

### Other Notable Changes

- #12433: Fixes the searchable annotations in the model supporting `Dashboard` to `Dashboard` lineage within the `DashboardInfo` aspect. Users of Sigma and PowerBI Apps ingestion may need to run a targeted [reindex](https://datahubproject.io/docs/how/restore-indices/) to update metadata on these edges.

## 0.15.0

- OpenAPI Update: PIT Keep Alive parameter added to scroll endpoints. NOTE: This parameter requires the `pointInTimeCreationEnabled` feature flag to be enabled and the `elasticSearch.implementation` configuration to be `elasticsearch`. This feature is not supported for OpenSearch at this time and the parameter will not be respected without both of these set.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.linkedin.common.Edge;
import com.linkedin.common.urn.ChartUrn;
import com.linkedin.common.urn.DashboardUrn;
import com.linkedin.common.urn.DatasetUrn;
import com.linkedin.common.urn.Urn;
import com.linkedin.metadata.aspect.patch.PatchOperationType;
Expand All @@ -19,6 +20,7 @@
extends AbstractMultiFieldPatchBuilder<DashboardInfoPatchBuilder> {
private static final String CHART_EDGES_PATH_START = "/chartEdges/";
private static final String DATASET_EDGES_PATH_START = "/datasetEdges/";
private static final String DASHBOARDS_PATH_START = "/dashboards/";

// Simplified with just Urn
public DashboardInfoPatchBuilder addChartEdge(@Nonnull ChartUrn urn) {
Expand All @@ -36,6 +38,7 @@
return this;
}

// Simplified with just Urn
public DashboardInfoPatchBuilder addDatasetEdge(@Nonnull DatasetUrn urn) {
ObjectNode value = createEdgeValue(urn);

Expand All @@ -52,6 +55,22 @@
return this;
}

// Simplified with just Urn
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add tests for these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found these tests for DashboardInfoPatchBuilder

I will add it there, even if they are all @Ignore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in commit 49e0dc2

public DashboardInfoPatchBuilder addDashboard(@Nonnull DashboardUrn urn) {
ObjectNode value = createEdgeValue(urn);

Check warning on line 60 in entity-registry/src/main/java/com/linkedin/metadata/aspect/patch/builder/DashboardInfoPatchBuilder.java

View check run for this annotation

Codecov / codecov/patch

entity-registry/src/main/java/com/linkedin/metadata/aspect/patch/builder/DashboardInfoPatchBuilder.java#L60

Added line #L60 was not covered by tests

pathValues.add(
ImmutableTriple.of(PatchOperationType.ADD.getValue(), DASHBOARDS_PATH_START + urn, value));
return this;

Check warning on line 64 in entity-registry/src/main/java/com/linkedin/metadata/aspect/patch/builder/DashboardInfoPatchBuilder.java

View check run for this annotation

Codecov / codecov/patch

entity-registry/src/main/java/com/linkedin/metadata/aspect/patch/builder/DashboardInfoPatchBuilder.java#L62-L64

Added lines #L62 - L64 were not covered by tests
}

public DashboardInfoPatchBuilder removeDashboard(@Nonnull DashboardUrn urn) {
pathValues.add(
ImmutableTriple.of(
PatchOperationType.REMOVE.getValue(), DASHBOARDS_PATH_START + urn, null));
return this;

Check warning on line 71 in entity-registry/src/main/java/com/linkedin/metadata/aspect/patch/builder/DashboardInfoPatchBuilder.java

View check run for this annotation

Codecov / codecov/patch

entity-registry/src/main/java/com/linkedin/metadata/aspect/patch/builder/DashboardInfoPatchBuilder.java#L68-L71

Added lines #L68 - L71 were not covered by tests
}

// Full Edge modification
public DashboardInfoPatchBuilder addEdge(@Nonnull Edge edge) {
ObjectNode value = createEdgeValue(edge);
Expand Down Expand Up @@ -87,6 +106,10 @@
return CHART_EDGES_PATH_START + destinationUrn;
}

if (DASHBOARD_ENTITY_NAME.equals(destinationUrn.getEntityType())) {
return DASHBOARDS_PATH_START + destinationUrn;

Check warning on line 110 in entity-registry/src/main/java/com/linkedin/metadata/aspect/patch/builder/DashboardInfoPatchBuilder.java

View check run for this annotation

Codecov / codecov/patch

entity-registry/src/main/java/com/linkedin/metadata/aspect/patch/builder/DashboardInfoPatchBuilder.java#L110

Added line #L110 was not covered by tests
}

// TODO: Output Data Jobs not supported by aspect, add here if this changes

throw new IllegalArgumentException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class DashboardInfoTemplate implements ArrayMergingTemplate<DashboardInfo
private static final String DATASETS_FIELD_NAME = "datasets";
private static final String CHARTS_FIELD_NAME = "charts";
private static final String DESTINATION_URN_FIELD_NAME = "destinationUrn";
private static final String DASHBOARDS_FIELD_NAME = "dashboards";

@Override
public DashboardInfo getSubtype(RecordTemplate recordTemplate) throws ClassCastException {
Expand Down Expand Up @@ -74,6 +75,12 @@ public JsonNode transformFields(JsonNode baseNode) {
DATASET_EDGES_FIELD_NAME,
Collections.singletonList(DESTINATION_URN_FIELD_NAME));

transformedNode =
Copy link
Collaborator

Choose a reason for hiding this comment

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

If there isn't already a test for this let's create one, or if there is add in mapping this field

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had already added a test for DashboardInfoTemplate in https://github.com/datahub-project/datahub/pull/12433/files#diff-b6808c495535fbf32144cbc4e6f16eb762b55c377c63ef929758d21760c28437

The test operates on applyPatch method, which goes through both transformFields and rebaseFields methods, as can be seen in

default T applyPatch(RecordTemplate recordTemplate, JsonPatch jsonPatch)
throws JsonProcessingException {
TemplateUtil.validatePatch(jsonPatch);
JsonNode transformed = populateTopLevelKeys(preprocessTemplate(recordTemplate), jsonPatch);
try {
// Hack in a more efficient patcher. Even with the serialization overhead 140% faster
JsonObject patched =
jsonPatch.apply(
Json.createReader(new StringReader(OBJECT_MAPPER.writeValueAsString(transformed)))
.readObject());
JsonNode postProcessed = rebaseFields(OBJECT_MAPPER.readTree(patched.toString()));
return RecordUtils.toRecordTemplate(getTemplateType(), postProcessed.toString());
} catch (JsonProcessingException e) {
throw new RuntimeException(
String.format(
"Error performing JSON PATCH on aspect %s. Patch: %s Target: %s",
recordTemplate.schema().getName(), jsonPatch, transformed.toString()),
e);
}
}
/**
* Returns a json representation of the template, modified for template based operations to be
* compatible with patch semantics.
*
* @param recordTemplate template to be transformed into json
* @return a {@link JsonNode} representation of the template
* @throws JsonProcessingException if there is an issue converting the input to JSON
*/
default JsonNode preprocessTemplate(RecordTemplate recordTemplate)
throws JsonProcessingException {
T subtype = getSubtype(recordTemplate);
JsonNode baseNode = OBJECT_MAPPER.readTree(RecordUtils.toJsonString(subtype));
return transformFields(baseNode);
}

arrayFieldToMap(
transformedNode,
DASHBOARDS_FIELD_NAME,
Collections.singletonList(DESTINATION_URN_FIELD_NAME));

transformedNode =
arrayFieldToMap(transformedNode, DATASETS_FIELD_NAME, Collections.emptyList());

Expand All @@ -97,6 +104,12 @@ public JsonNode rebaseFields(JsonNode patched) {
CHART_EDGES_FIELD_NAME,
Collections.singletonList(DESTINATION_URN_FIELD_NAME));

rebasedNode =
transformedMapToArray(
rebasedNode,
DASHBOARDS_FIELD_NAME,
Collections.singletonList(DESTINATION_URN_FIELD_NAME));

rebasedNode = transformedMapToArray(rebasedNode, DATASETS_FIELD_NAME, Collections.emptyList());
rebasedNode = transformedMapToArray(rebasedNode, CHARTS_FIELD_NAME, Collections.emptyList());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,26 @@ public void testDashboardInfoTemplate() throws Exception {
UrnUtils.getUrn("urn:li:dataset:(urn:li:dataPlatform:hive,SampleHiveDataset,PROD)"),
result.getDatasetEdges().get(0).getDestinationUrn());
}

@Test
public void testDashboardInfoTemplateDashboardsField() throws Exception {
DashboardInfoTemplate dashboardInfoTemplate = new DashboardInfoTemplate();
DashboardInfo dashboardInfo = dashboardInfoTemplate.getDefault();
JsonPatchBuilder jsonPatchBuilder = Json.createPatchBuilder();
jsonPatchBuilder.add(
"/dashboards/urn:li:dashboard:(urn:li:dataPlatform:tableau,SampleDashboard,PROD)",
Json.createObjectBuilder()
.add(
"destinationUrn",
Json.createValue(
"urn:li:dashboard:(urn:li:dataPlatform:tableau,SampleDashboard,PROD)"))
.build());

DashboardInfo result =
dashboardInfoTemplate.applyPatch(dashboardInfo, jsonPatchBuilder.build());

Assert.assertEquals(
UrnUtils.getUrn("urn:li:dashboard:(urn:li:dataPlatform:tableau,SampleDashboard,PROD)"),
result.getDashboards().get(0).getDestinationUrn());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ def convert_dashboard_info_to_patch(
if aspect.datasets:
patch_builder.add_datasets(aspect.datasets)

if aspect.dashboards:
for dashboard in aspect.dashboards:
patch_builder.add_dashboard(dashboard)

if aspect.access:
patch_builder.set_access(aspect.access)

Expand Down
44 changes: 43 additions & 1 deletion metadata-ingestion/src/datahub/specific/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@
lastModified=self._mint_auditstamp(),
)

self._ensure_urn_type("dataset", [chart_edge], "add_chart_edge")
self._ensure_urn_type("chart", [chart_edge], "add_chart_edge")
self._add_patch(
DashboardInfo.ASPECT_NAME,
"add",
Expand Down Expand Up @@ -271,6 +271,48 @@

return self

def add_dashboard(
self, dashboard: Union[Edge, Urn, str]
) -> "DashboardPatchBuilder":
"""
Adds an dashboard to the DashboardPatchBuilder.

Args:
dashboard: The dashboard, which can be an Edge object, Urn object, or a string.

Returns:
The DashboardPatchBuilder instance.

Raises:
ValueError: If the dashboard is not a Dashboard urn.

Notes:
If `dashboard` is an Edge object, it is used directly. If `dashboard` is a Urn object or string,
it is converted to an Edge object and added with default audit stamps.
"""
if isinstance(dashboard, Edge):
dashboard_urn: str = dashboard.destinationUrn
dashboard_edge: Edge = dashboard
elif isinstance(dashboard, (Urn, str)):
dashboard_urn = str(dashboard)
if not dashboard_urn.startswith("urn:li:dashboard:"):
raise ValueError(f"Input {dashboard} is not a Dashboard urn")

Check warning on line 299 in metadata-ingestion/src/datahub/specific/dashboard.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/specific/dashboard.py#L296-L299

Added lines #L296 - L299 were not covered by tests

dashboard_edge = Edge(

Check warning on line 301 in metadata-ingestion/src/datahub/specific/dashboard.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/specific/dashboard.py#L301

Added line #L301 was not covered by tests
destinationUrn=dashboard_urn,
created=self._mint_auditstamp(),
lastModified=self._mint_auditstamp(),
)

self._ensure_urn_type("dashboard", [dashboard_edge], "add_dashboard")
self._add_patch(
DashboardInfo.ASPECT_NAME,
"add",
path=("dashboards", dashboard_urn),
value=dashboard_edge,
)
return self

def set_dashboard_url(
self, dashboard_url: Optional[str]
) -> "DashboardPatchBuilder":
Expand Down
Loading
Loading