Skip to content

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

Open
vaibhavatlan wants to merge 2 commits into
mainfrom
feat/metabase-creators
Open

feat: add creator() factories for the Metabase asset family#975
vaibhavatlan wants to merge 2 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.

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.

1 participant