fix(apps): send the full existing connection on a miner run (AICHAT-1798) - #1037
Merged
Conversation
…798) A miner references an existing connection by qualifiedName, but _create() sent a stub carrying only the QN and connectorName — no name. atlan-popularity-app then derives the connection name from an empty value, falls back to the qualifiedName's numeric tail, and publish renames the connection to that number. The UI and a rerun never hit this because they send the whole connection. _create() now reads the connection back in full (get_by_qualified_name) and sends the entire object the same way a rerun does, so a full-replace downstream drops no attributes. The read-back carries the connection's own defaultCredentialGuid, so the credential is still reused. If the connection cannot be read and no explicit name was given, it raises CONNECTION_READ_FOR_APP_FAILED rather than silently sending a stub that would rename the connection.
…AICHAT-1798) The read-back used min_ext_info=False, which returns the connection's entire attribute set — ~89 fields including empty analytics arrays and computed scores (popularityScore, viewScore, sourceRead*, assetMc*, ...). Echoing those from an extract step is noise and can clobber real popularity on a full-replace. Fetch only the config attributes the UI forwards on a miner run (identity, credential/policy strategy, admins, query settings) via get_by_qualified_name's attributes= list. The payload now matches the frontend's connection shape (~21 config fields, name included), with no analytics/popularity fields.
…lytics Add a client-side allowlist filter so no attribute outside _CONNECTION_WIRE_ATTRS can ride on the payload even if the read returns more, and strengthen the miner test: it now asserts the read requests only the curated attributes, the config fields (name/category/rowLimit/admins/connectorName/defaultCredentialGuid) flow through, and computed analytics on the read-back (popularityScore/viewScore) are dropped rather than forwarded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A pyatlan-created BigQuery/Snowflake/etc. miner that references an existing connection by qualifiedName only was renaming that connection to its qualifiedName's numeric tail (e.g.
bigquery-tvf-line-2→1788198662).Closes AICHAT-1798.
Root cause
_create()sent a stub connection —qualifiedName+connectorName, noname.atlan-popularity-appderives the connection name from that payload; with the name empty it falls back toconnection_qualified_name.rsplit("/")[-1](the numeric tail) and publish writes it back.Why only some runs: the UI native run and an SDK update/rerun (
load()) send the whole connection (name included) via_loaded_connection, so they were never affected. Only a freshrun()with a QN-only reference hit the stub path.Fix
A fresh miner run now reads the connection back in full (
get_by_qualified_name(..., min_ext_info=False)) and sends the entire object the same way a rerun does — routed through the existing full-send path — so a full-replace downstream drops no attribute (not justname; alsocategory/rowLimit/etc.). The read-back carries the connection's owndefaultCredentialGuid, so the credential is still reused with no separate lookup (top-levelcredential_guidstays"").If the connection cannot be read and no explicit
connection(name=...)was given, it raisesCONNECTION_READ_FOR_APP_FAILED(ATLAN-PYTHON-400-081) instead of silently sending a stub that would rename the connection.Scope & safety
name) and staged-credential paths never trigger it..json()so set-typed attributes are JSON-safe (a.dict()would emit raw sets)..connection(name=…/admin_*=…)/.credential_guid()still override the read-back.Tests
tests/unit/test_app_builders.py— full read-back sent; connector derived from QN; explicit-name override; read-failure raises; explicit-name recovers a read failure. 449 passed, 3 skipped, ruff clean.