Skip to content

Fixes #27040: Guard postDelete() with hardDelete check in IngestionPipeline, DataContract, App #27043

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.

Sign up for GitHub

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

Open
RajdeepKushwaha5 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixes #27040: Guard postDelete() with hardDelete check in IngestionPipeline, DataContract, App #27043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 8 additions & 6 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/AppRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,14 @@ public void storeRelationships(App entity) {
@Override
protected void postDelete(App entity, boolean hardDelete) {
super.postDelete(entity, hardDelete);
// Delete the status stored in the app extension
// Note that we don't want to delete the LIMITS, since we want to keep them
// between different app installations
daoCollection
.appExtensionTimeSeriesDao()
.delete(entity.getId().toString(), AppExtension.ExtensionType.STATUS.toString());
if (hardDelete) {
// Delete the status stored in the app extension
// Note that we don't want to delete the LIMITS, since we want to keep them
// between different app installations
daoCollection
.appExtensionTimeSeriesDao()
.delete(entity.getId().toString(), AppExtension.ExtensionType.STATUS.toString());
}
Comment on lines 249 to +258
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

Consider adding a test to ensure that on soft delete (hardDelete=false) the app run STATUS time series is preserved (so restore brings back run history), while hard delete still cleans it up. This guards the regression this PR is addressing.

Copilot uses AI. Check for mistakes.
}

public final List<App> listAll() {
Expand Down
14 changes: 8 additions & 6 deletions ...metadata-service/src/main/java/org/openmetadata/service/jdbi3/DataContractRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,15 @@ protected void postUpdate(DataContract original, DataContract updated) {
@Override
protected void postDelete(DataContract dataContract, boolean hardDelete) {
super.postDelete(dataContract, hardDelete);
if (!nullOrEmpty(dataContract.getQualityExpectations())) {
deleteTestSuite(dataContract);
if (hardDelete) {
if (!nullOrEmpty(dataContract.getQualityExpectations())) {
deleteTestSuite(dataContract);
}
// Clean status
daoCollection
.entityExtensionTimeSeriesDao()
.delete(dataContract.getFullyQualifiedName(), RESULT_EXTENSION);
}
Comment on lines 241 to 251
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

The PR description states a test was added for this scenario, but I couldn’t find any new/updated test referencing this behavior. Either add a test that proves soft delete preserves DataContract result time series + test suite (and hard delete removes them), or update the PR description/checklist to reflect the current state.

Copilot uses AI. Check for mistakes.
// Clean status
daoCollection
.entityExtensionTimeSeriesDao()
.delete(dataContract.getFullyQualifiedName(), RESULT_EXTENSION);
}

private void postCreateOrUpdate(DataContract dataContract) {
Expand Down
24 changes: 13 additions & 11 deletions ...ata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,18 +474,20 @@ public EntityRepository<IngestionPipeline>.EntityUpdater getUpdater(
@Override
protected void postDelete(IngestionPipeline entity, boolean hardDelete) {
super.postDelete(entity, hardDelete);
// Delete deployed pipeline in the Pipeline Service Client
if (pipelineServiceClient != null) {
pipelineServiceClient.deletePipeline(entity);
} else {
LOG.debug(
"Skipping pipeline service delete for '{}' because pipeline service client is not configured.",
entity.getFullyQualifiedName());
if (hardDelete) {
// Delete deployed pipeline in the Pipeline Service Client
if (pipelineServiceClient != null) {
pipelineServiceClient.deletePipeline(entity);
} else {
LOG.debug(
"Skipping pipeline service delete for '{}' because pipeline service client is not configured.",
entity.getFullyQualifiedName());
}
// Clean pipeline status
daoCollection
.entityExtensionTimeSeriesDao()
.delete(entity.getFullyQualifiedName(), PIPELINE_STATUS_EXTENSION);
}
Comment on lines 475 to 490
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

There’s no test coverage validating that a soft delete (hardDelete=false) no longer triggers the pipeline service deletion or the PIPELINE_STATUS time series cleanup. Since this change is meant to prevent irreversible data loss on soft-delete→restore, add a unit/integration test that (a) performs soft delete + restore and asserts time series entries remain, and (b) verifies the hard delete path still removes them.

Copilot uses AI. Check for mistakes.
// Clean pipeline status
daoCollection
.entityExtensionTimeSeriesDao()
.delete(entity.getFullyQualifiedName(), PIPELINE_STATUS_EXTENSION);
}

@Override
Expand Down
Loading