Skip to content

feat: add creator() factories for the Metabase asset family#975

Merged
Aryamanz29 merged 4 commits into
mainfrom
feat/metabase-creators
Jul 22, 2026
Merged

feat: add creator() factories for the Metabase asset family#975
Aryamanz29 merged 4 commits into
mainfrom
feat/metabase-creators

Conversation

@vaibhavatlan

Copy link
Copy Markdown
Contributor

Problem

MetabaseCollection, MetabaseDashboard, and MetabaseQuestion ship without a .creator()/.create() factory (they only have create_for_modification). Because there's no factory that owns the qualifiedName grammar, connectors hand-build Metabase qualifiedNames with f-strings — duplicating Atlan's QN grammar per connector. That's exactly the anti-pattern the application-sdk conformance rule P028 ManualQualifiedNameFString flags (11 live warnings on one connector today, unresolvable in the app because no upstream factory exists).

Change

Adds creators that own the grammar centrally:

Asset creator(...) qualifiedName
MetabaseCollection name, connection_qualified_name, metabase_id {connection_qualified_name}/collections/{metabase_id}
MetabaseDashboard name, connection_qualified_name, metabase_id {connection_qualified_name}/dashboards/{metabase_id}
MetabaseQuestion name, connection_qualified_name, metabase_id {connection_qualified_name}/questions/{metabase_id}

Each follows the standard pattern: creator() + a deprecated create() alias on the asset, and an Attributes.create() that builds the QN and derives connector_name.

Design note — id-based grammar (for maintainer review)

The house convention builds QN from the name ({connection_qualified_name}/{name}). The Metabase creators instead use an id-based grammar with a typed segment (/collections/{metabase_id}), because that is the qualifiedName connectors already write in production — a name-based grammar would not match existing assets and would break dedup/lineage on adoption. This PR effectively establishes the canonical Metabase QN grammar; flagging it explicitly in case maintainers prefer a different shape.

What's included

  • Generator method templates (source of truth): methods/asset/metabase_{collection,dashboard,question}.jinja2 + methods/attribute/....
  • The regenerated creator methods on the three asset classes (+ required imports).
  • Parametrized unit tests per asset: creator, deprecated-create warning, missing-parameter validation, and create_for_modification / trim_to_required. 24 tests, all passing; ruff check clean; black-formatted.

Notes

  • The metabase_id is typed str (callers stringify the Metabase system id).
  • Left HISTORY.md untouched — it looks release-curated; happy to add an entry in whatever form you prefer.
  • Collection/dashboard relationship wiring (e.g. a dashboard's parent collection) is intentionally out of scope for this first cut; creators are connection-rooted. Can add optional relationship params in a follow-up.

MetabaseCollection, MetabaseDashboard, and MetabaseQuestion shipped without a
.creator()/.create() factory, so connectors hand-build their qualifiedNames with
f-strings — duplicating Atlan's QN grammar per connector (the exact anti-pattern
the application-sdk conformance rule P028 flags).

Add creators that own the grammar centrally:
- MetabaseCollection.creator(name, connection_qualified_name, metabase_id)
    -> {connection_qualified_name}/collections/{metabase_id}
- MetabaseDashboard.creator(...) -> {connection_qualified_name}/dashboards/{metabase_id}
- MetabaseQuestion.creator(...)  -> {connection_qualified_name}/questions/{metabase_id}

The grammar is id-based (a Metabase system id in a typed segment) to match the
qualifiedNames connectors already write in production; a name-based grammar would
not match existing assets. This is a departure from the name-based house
convention and is called out for maintainer review.

Adds the generator method templates (methods/asset + methods/attribute) as the
source of truth, the regenerated asset methods, and parametrized creator tests
(creator, deprecated create warning, missing-param validation, modification).
Mirror the pyatlan Metabase creators into pyatlan_v9 (the package the metabase
connector actually imports). v9 uses the _overlays mechanism + flat asset
classes, so this adds:
- _overlays/metabase_{collection,dashboard,question}.py (creator source of truth)
- the merged creator/create/create_for_modification on the three flat asset
  classes, importing init_guid/validate_required_fields from pyatlan_v9.utils

Same id-based, production-matching grammar as the pyatlan side
({connection_qualified_name}/collections/{metabase_id}, etc.). connector_name is
derived by splitting the connection qualified name, matching the v9 house pattern
(e.g. PresetWorkspace).
@linear

linear Bot commented Jul 17, 2026

Copy link
Copy Markdown

BLDX-1558

@vaibhavatlan

Copy link
Copy Markdown
Contributor Author

Update: mirrored into pyatlan_v9 + generation verification

pyatlan_v9 now included (the package the metabase connector actually imports). v9 uses the _overlays/ mechanism + flat asset classes, so this adds:

  • pyatlan_v9/model/assets/_overlays/metabase_{collection,dashboard,question}.py (creator source of truth)
  • the merged creator/create/create_for_modification on the three flat asset classes (imports from pyatlan_v9.utils; connector_name derived by splitting the connection QN, matching the v9 PresetWorkspace pattern)

Same id-based, production-matching grammar on both sides. Verified functionally on both packages:

pyatlan   : default/metabase/1/collections/9
pyatlan_v9: default/metabase/1/collections/9

"Nothing else changes" verification (pyatlan generator): module.jinja2 resolves per-asset method templates by asset name — methods/asset/<entity_def.name | to_snake_case>.jinja2. The six new templates are metabase-named only, so regeneration can affect only the three Metabase asset files; no other asset's rendering can change. A full regenerate against real typedefs still needs a tenant (ATLAN_BASE_URL/ATLAN_API_KEY); the committed tests/unit/data/typedefs.json is a reduced fixture and is not suitable for a full clean regen.

vaibhavatlan and others added 2 commits July 22, 2026 12:33
Running the SDK generator against a live tenant reproduces the three
Metabase creator files byte-identically from the jinja2 templates, except
for one stray blank line between the atlan_fields import block and the
pyatlan.utils import in metabase_collection.py that the generator does not
emit. Drop it so the committed file matches true generator output.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Connection-rooted integration coverage for MetabaseCollection/Dashboard/
Question .creator() factories: seeds one of each under a live Metabase
connection, asserts the id-based qualifiedName grammar
({connection_qualified_name}/{collections,dashboards,questions}/{metabase_id})
and connector_name, then exercises certificate/announcement update,
retrieve, delete and restore. Verified green against a real tenant
(7 passed).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vaibhavatlan

Copy link
Copy Markdown
Contributor Author

Verified against a real tenant

Generator (source-of-truth check). Ran uv run ./generator --override against a live tenant (which ships all three Metabase entity defs). The three Metabase asset files regenerate from the .jinja2 templates as follows:

  • metabase_dashboard.py, metabase_question.pybyte-identical to what's committed.
  • metabase_collection.py → identical except one stray blank line in the imports that the generator does not emit — fixed in 0643308.

All creator / Attributes.create bodies come straight from the templates and reproduce exactly, so the templates are confirmed as the source of truth. (The rest of the generator diff was unrelated typedef drift from a newer tenant baseline and was discarded.)

Integration tests (0fbcb36). Added tests/integration/metabase_asset_test.py — connection-rooted coverage for all three creators: seeds one of each under a live Metabase connection, asserts the id-based qualifiedName grammar + connector_name, then exercises certificate/announcement update, retrieve, delete and restore.

7 passed in 77.24s

Confirms the assets actually seed and round-trip on a real tenant.

@Aryamanz29 Aryamanz29 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM - thanks!

@Aryamanz29
Aryamanz29 merged commit 45c0c32 into main Jul 22, 2026
64 of 76 checks passed
@Aryamanz29 Aryamanz29 added the feature New feature or request label Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants