Skip to content

fix(lake-formation-tag-sync): option to keep connection key in schema name - #2802

Merged
jtgatlan merged 2 commits into
mainfrom
christophertin/csa-628
Oct 6, 2026
Merged

jtgatlan merged 2 commits into
mainfrom
christophertin/csa-628

Conversation

@christopher-tin-atlan

@christopher-tin-atlan christopher-tin-atlan commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

What

Lake Formation Tag Sync builds each schema qualifiedName from the Lake Formation DatabaseName with its first _-separated segment (the connection_map.json key) stripped off: prod_sales_analytics → key prod, schema sales_analytics.

That's correct when the crawled schema name excludes the prefix. When the crawled schema keeps it (prod_sales_analytics), every schema, table and column lookup misses and is skipped as not found in update-only mode. No connection_map.json value can work around it: the code always puts / between the mapped connection and the stripped remainder, and an empty-string key can never match.

Separately, a DatabaseName with no _ throws IndexOutOfBoundsException when the split result is destructured.

How

  • New opt-in input keep_database_prefix ("Keep connection key in schema name"), default false. When true, the first segment still resolves the connection, but the full DatabaseName is used as the schema name. remove_schema strips whichever schema name is in use.
  • A DatabaseName with no _ is logged and skipped instead of throwing.
  • Help text: fixed the "Drom" typo on remove_schema and gave both schema-name options a concrete example; the keep_database_prefix help states the default (the connection_map.json key is stripped from the schema name). Labels and keys are unchanged, but the remove_schema help text changes for every tenant.
  • LakeFormationTagSyncCfg.kt regenerated from package.pkl via genCustomPkg.

Default behaviour is unchanged, so existing tenant configurations that rely on the stripping keep working.

Testing

New CSVProducerSchemaNameTest (no tenant needed), 4/4 passing locally on JDK 17:

  • default still strips the key from the schema
  • flag on uses the full DatabaseName for schema, table and column qualifiedNames
  • flag on with remove_schema strips the full name from the table name
  • no-underscore DatabaseName is skipped, not thrown

Existing tenant-backed tests (CSVProducerTest, LakeTagSynchronizerTest, EnumCreatorTest) were not run locally.

Follow-up

After release, bump the csa-lake-formation-tag-sync image and add the new field in marketplace-packages/packages/csa/lake-formation-tag-sync.

Refs CSA-628

🤖 Generated with Claude Code

… name

The schema segment of every qualifiedName is the Lake Formation
DatabaseName with its first "_"-separated segment (the connection_map
key) stripped off. Where the crawled schema name keeps that prefix,
every schema, table and column lookup misses and is skipped as not
found, and no connection_map.json value can restore the prefix.

Add an opt-in keep_database_prefix input (default false). When set, the
first segment is still used to resolve the connection, but the full
DatabaseName is used as the schema name. Existing configurations are
unaffected.

Also skip and log a DatabaseName with no "_" instead of throwing
IndexOutOfBoundsException when destructuring the split.

Refs CSA-628

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear

linear Bot commented Oct 5, 2026

Copy link
Copy Markdown

CSA-628

Fix the "Drom" typo in the remove_schema help text and give both
schema-name options a concrete example. The keep_database_prefix help
now states the default behaviour (the connection key is stripped from
the schema name) so it is clear when to turn it on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@christopher-tin-atlan

Copy link
Copy Markdown
Collaborator Author

End-to-end verification

Tested on an internal Atlan tenant with a branch image built through the manual merge.yml run (7.4.2-SNAPSHOT-christophertin-csa-628). The feed had two association files against one connection:

  • key_schema_a, where the crawled schema name keeps the key prefix;
  • otherkey_key_schema_a, the old shape, where the key is stripped.
Run 1: option absent from the saved workflow Run 2: keep_database_prefix: true
NESTED_CONFIG "keep_database_prefix": false, from the template default "keep_database_prefix": true
Prefix kept in the crawled schema name Looked up …/schema_a, 33 skipped (reproduces the bug) Looked up …/key_schema_a, 1 schema + 1 table + 31 columns updated, 0 skipped
Old shape 33 matched, unchanged Skipped, as expected with the option on

After run 2 I confirmed in the catalog that all 33 assets carry the value written by the run.

Backward compatibility: a workflow saved before this input existed ran unchanged. Argo applied the template default, and LakeFormationTagSyncCfg defaults to false when the key is missing.

Unrelated issue found while testing: CustomMetadataCache fails to load when any custom-metadata attribute on the tenant has options.primitiveType outside AtlanCustomAttributePrimitiveType (e.g. "text"):

InvalidFormatException: Cannot deserialize value of type AtlanCustomAttributePrimitiveType from String "text"

That stops every package that loads the cache. Not addressed here; I'll raise it separately.

@christopher-tin-atlan

Copy link
Copy Markdown
Collaborator Author

@jtgatlan

jtgatlan commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Nice work on this, Tin. The fix is clean and keeping it opt-in protects existing configs.

One thing before merge: the green pr-test check doesn't actually cover the new CSVProducerSchemaNameTest. Package tests only run when -PpackageTests is set (see buildSrc/src/main/kotlin/com.atlan.kotlin-custom-package.gradle.kts), so CI skipped it. Could you paste the output of your local run here, or run:

./gradlew :samples:packages:lake-formation-tag-sync:test --tests "com.atlan.pkg.lftag.CSVProducerSchemaNameTest" -PpackageTests

That way the PR has a record that the tests passed. Thanks!

@christopher-tin-atlan

Copy link
Copy Markdown
Collaborator Author

Thanks John, good catch on -PpackageTests. Here's a fresh local run of that command against the PR head (019a812f8d), with --rerun-tasks so nothing came from cache. JDK 17 (OpenJDK 17.0.20.1):

$ ./gradlew :samples:packages:lake-formation-tag-sync:test --tests "com.atlan.pkg.lftag.CSVProducerSchemaNameTest" -PpackageTests --rerun-tasks

com.atlan.pkg.lftag.CSVProducerSchemaNameTest databaseNameWithoutUnderscoreIsSkippedNotThrown PASSED
com.atlan.pkg.lftag.CSVProducerSchemaNameTest defaultStripsConnectionKeyFromSchema PASSED
com.atlan.pkg.lftag.CSVProducerSchemaNameTest keepDatabasePrefixUsesFullDatabaseNameAsSchema PASSED
com.atlan.pkg.lftag.CSVProducerSchemaNameTest keepDatabasePrefixWithRemoveSchemaStripsFullDatabaseName PASSED
SUCCESS: Executed 4 tests in 911ms
BUILD SUCCESSFUL in 2m 3s

These tests don't need a tenant: they call CSVProducer.transform() directly and check the qualifiedNames it writes. The existing tenant-backed tests (CSVProducerTest, LakeTagSynchronizerTest, EnumCreatorTest) weren't run locally; the end-to-end runs on the test tenant above cover that path.

@jtgatlan jtgatlan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks Tin, the local run plus the end-to-end runs on the test tenant (including the backward-compat check) give us everything we need. Approving.

Two small things for after merge: the marketplace-packages bump so the new option shows up for users, and a Linear ticket for the CustomMetadataCache primitiveType issue you found, since that one can stop every package on an affected tenant. Great work on this.

@jtgatlan
jtgatlan merged commit 39c9730 into main Oct 6, 2026
23 checks passed
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.

2 participants