Skip to content

ignore deleting core types. log errors when it happens #5407

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
wants to merge 2 commits into
base: staging
Choose a base branch
from
Open

ignore deleting core types. log errors when it happens #5407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce72516
ignore deleting core types. log errors when it happens
sriram-atlan Sep 19, 2025
5557083
branch to build
sriram-atlan Sep 19, 2025
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
1 change: 1 addition & 0 deletions .github/workflows/maven.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ on:
- beta
- master
- staging
- typdef-ignore-atlas-core

jobs:
build:
Expand Down
32 changes: 30 additions & 2 deletions repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,25 @@ public void init() throws AtlasBaseException {
@Override
public void reloadEnumTypeDefs() throws AtlasBaseException {
LOG.info("==> AtlasTypeDefGraphStore.reloadEnumTypeDefs()");
AtlasTransientTypeRegistry ttr = null;
boolean commitUpdates = false;
AtlasTransientTypeRegistry ttr = null;
boolean commitUpdates = false;

try {
ttr = typeRegistry.lockTypeRegistryForUpdate(5);
List<AtlasEnumDef> enumDefs = getEnumDefStore(ttr).getAll();
for (AtlasBaseTypeDef atlasBaseTypeDef : ttr.getAllEnumDefs()) {
if (atlasBaseTypeDef instanceof AtlasEnumDef) {
if ("atlas_core".equalsIgnoreCase(atlasBaseTypeDef.getServiceType())) {
LOG.debug("Skipping removal of built-in enum type: {}", atlasBaseTypeDef.getName());
continue;
}
ttr.removeTypeByName(atlasBaseTypeDef.getName());
}
}
ttr.addTypes(enumDefs);
commitUpdates = true;
} catch (Throwable abe) {
LOG.warn("reloadEnumTypeDefs(): failed to reload enum defs", abe);
Copy link

Choose a reason for hiding this comment

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

Bug: API Contract Violation in Exception Handling

The reload*TypeDefs methods now catch all Throwable exceptions and log them as warnings without rethrowing. This breaks the API contract, as these methods are declared to throw AtlasBaseException, masking critical errors and potentially leaving type definitions in a stale state.

Additional Locations (4)

Fix in Cursor Fix in Web

} finally {
typeRegistry.releaseTypeRegistryForUpdate(ttr, commitUpdates);
LOG.info("<== AtlasTypeDefGraphStore.reloadEnumTypeDefs()");
Expand All @@ -155,11 +161,17 @@ public void reloadStructTypeDefs() throws AtlasBaseException {
List<AtlasStructDef> structDefs = getStructDefStore(ttr).getAll();
for (AtlasBaseTypeDef atlasBaseTypeDef : ttr.getAllStructDefs()) {
if (atlasBaseTypeDef instanceof AtlasStructDef) {
if ("atlas_core".equalsIgnoreCase(atlasBaseTypeDef.getServiceType())) {
LOG.debug("Skipping removal of built-in struct type: {}", atlasBaseTypeDef.getName());
continue;
}
ttr.removeTypeByName(atlasBaseTypeDef.getName());
}
}
ttr.addTypes(structDefs);
commitUpdates = true;
} catch (Throwable abe) {
LOG.warn("reloadStructTypeDefs(): failed to reload struct defs", abe);
} finally {
typeRegistry.releaseTypeRegistryForUpdate(ttr, commitUpdates);
LOG.info("<== AtlasTypeDefGraphStore.reloadStructTypeDefs()");
Expand All @@ -177,11 +189,17 @@ public void reloadEntityTypeDefs() throws AtlasBaseException {
List<AtlasEntityDef> entityDefs = getEntityDefStore(ttr).getAll();
for (AtlasBaseTypeDef atlasBaseTypeDef : ttr.getAllEntityDefs()) {
if (atlasBaseTypeDef instanceof AtlasEntityDef) {
if ("atlas_core".equalsIgnoreCase(atlasBaseTypeDef.getServiceType())) {
LOG.debug("Skipping removal of built-in entity type: {}", atlasBaseTypeDef.getName());
continue;
}
ttr.removeTypeByName(atlasBaseTypeDef.getName());
}
}
ttr.addTypes(entityDefs);
commitUpdates = true;
} catch (Throwable abe) {
LOG.warn("reloadEntityTypeDefs(): failed to reload entity defs", abe);
} finally {
typeRegistry.releaseTypeRegistryForUpdate(ttr, commitUpdates);
LOG.info("<== AtlasTypeDefGraphStore.reloadEntityTypeDefs()");
Expand All @@ -199,11 +217,17 @@ public void reloadRelationshipTypeDefs() throws AtlasBaseException {
List<AtlasRelationshipDef> relationshipDefs = getRelationshipDefStore(ttr).getAll();
for (AtlasBaseTypeDef atlasBaseTypeDef : ttr.getAllRelationshipDefs()) {
if (atlasBaseTypeDef instanceof AtlasRelationshipDef) {
if ("atlas_core".equalsIgnoreCase(atlasBaseTypeDef.getServiceType())) {
LOG.debug("Skipping removal of built-in relationship type: {}", atlasBaseTypeDef.getName());
continue;
}
ttr.removeTypeByName(atlasBaseTypeDef.getName());
}
}
ttr.addTypes(relationshipDefs);
commitUpdates = true;
} catch (Throwable abe) {
LOG.warn("reloadRelationshipTypeDefs(): failed to reload relationship defs", abe);
} finally {
typeRegistry.releaseTypeRegistryForUpdate(ttr, commitUpdates);
LOG.info("<== AtlasTypeDefGraphStore.reloadRelationshipTypeDefs()");
Expand All @@ -226,6 +250,8 @@ public void reloadBusinessMetadataTypeDefs() throws AtlasBaseException {
}
ttr.addTypes(businessMetadataDefs);
commitUpdates = true;
} catch (Throwable abe) {
LOG.warn("reloadBusinessMetadataTypeDefs(): failed to reload BM defs", abe);
Copy link

Choose a reason for hiding this comment

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

Bug: Metadata Reload Inconsistencies and Exception Handling

Two inconsistencies appear in the reload*TypeDefs methods:

  1. reloadBusinessMetadataTypeDefs() and reloadClassificationMetadataTypeDefs() are missing the atlas_core service type check, allowing built-in types to be removed during reload, unlike other type definitions.
  2. All reload*TypeDefs methods now catch and log Throwable, effectively swallowing exceptions and breaking the API contract that declares throws AtlasBaseException.
Additional Locations (1)

Fix in Cursor Fix in Web

} finally {
typeRegistry.releaseTypeRegistryForUpdate(ttr, commitUpdates);
LOG.info("<== AtlasTypeDefGraphStore.reloadBusinessMetadataTypeDefs()");
Expand All @@ -248,6 +274,8 @@ public void reloadClassificationMetadataTypeDefs() throws AtlasBaseException {
}
ttr.addTypes(classificationDefs);
commitUpdates = true;
} catch (Throwable abe) {
Copy link

Choose a reason for hiding this comment

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

Bug: Classification Metadata Reload Fails Core Type Check

The reloadClassificationMetadataTypeDefs() method is missing the atlas_core service type check. This allows built-in classification types to be removed during reload operations, which is inconsistent with other reload*TypeDefs methods that now protect their core types.

Fix in Cursor Fix in Web

LOG.warn("reloadClassificationMetadataTypeDefs(): failed to reload classification defs", abe);
} finally {
typeRegistry.releaseTypeRegistryForUpdate(ttr, commitUpdates);
LOG.info("<== AtlasTypeDefGraphStore.reloadClassificationMetadataTypeDefs()");
Expand Down
Loading