HYP-829: run publish preflight as first extract-workflow activity (user gate, no error codes)#1966
Draft
rupeshatlan wants to merge 14 commits into
Draft
HYP-829: run publish preflight as first extract-workflow activity (user gate, no error codes)#1966rupeshatlan wants to merge 14 commits into
rupeshatlan wants to merge 14 commits into
Conversation
…(HYP-829) - Add http_status property to FailureCategory enum with RFC-standard mappings (AUTH→401, PERMISSION→403, INVALID_INPUT→422, DEPENDENCY_UNAVAILABLE→503, TIMEOUT→504, INTERNAL→500, etc.) - Add http_status property to AppError that delegates to its category - Guard model_validate in auth/preflight/metadata endpoints so ValidationError raises InvalidInputError (422) instead of 500 - Add except AppError clause before bare except Exception in all three handler endpoints so typed errors (AuthError, NotFoundError, etc.) surface the correct HTTP status to callers
…HYP-829 G6)
Rewrites check_atlan_publish_permission to call Heracles endpoints
(GET /users/{id}, POST /evaluates) instead of PyAtlan, mirroring the
battle-tested marketplace-scripts/preflight/publish_checks.py approach.
Adds check_user_enabled as a standalone helper for the user-enabled check.
Both checks run in parallel via asyncio.gather — same pattern the call
discussion agreed on (source checks + user permission checks concurrently).
Adds PublishPreflightMixin (application_sdk/app/preflight.py) — a Temporal
activity mixin that apps inherit to get run_publish_preflight() as their
first workflow activity. This covers scheduled workflows, which bypass the
Heracles HTTP-preflight call and go directly to Temporal.
Checks performed (in parallel):
1. UserEnabled — GET /users/{user_id}; rejects disabled accounts
2. AtlanPublishPermission — POST /evaluates; confirms ENTITY_CREATE/UPDATE/DELETE
on the target connection via Keycloak token-exchange impersonation
Both skip gracefully (pass=True + warning log) when service credentials
or user_id are not available, so existing apps that haven't opted in yet
are unaffected.
Exports:
from application_sdk.handler import check_user_enabled, check_atlan_publish_permission
from application_sdk.app import PublishPreflightMixin
…n_publish_permission (HYP-829) - 11 tests covering enabled/disabled user, 403 from Heracles, network errors, permission granted/denied, skipped checks when creds absent, bad client secret - Fix PreflightOutput.checks field: use allow_unbounded_fields=True to pass payload safety validator (checks list is always small, <=2 items)
…(HYP-829) PreflightOutput skip-path instantiations were missing the checks kwarg, which `field(default_factory=list)` doesn't satisfy when combined with the dataclass + Pydantic Output + Annotated[..., MaxItems(20)] stack. Caught live in Hive worker logs: TypeError: missing 1 required positional argument: 'checks'. Also includes: - handler/checks.py — resolve HERACLES_URL at call time, not module import, so env vars injected after module load take effect (P7 follow-up). - handler/service.py — raise HTTPException(422) directly on model_validate failures so InvalidInputError doesn't escape unhandled to the test client / FastAPI. Adds except AppError clauses on /auth, /check, /metadata so typed errors return their semantic HTTP status code (HYP-829 EC). - tests/unit/errors/test_categorical.py — EC1+EC2: 14 cases for FailureCategory.http_status mapping + 6 cases for AppError.http_status delegation + guard test. - tests/unit/handler/test_service.py — EC3-EC6: AppError subclasses on each endpoint return semantic codes; malformed bodies return 422; no stack trace leakage in error detail.
… add unit tests (HYP-829) Root cause of the live Hive worker TypeError 'missing checks argument': combining @DataClass with a Pydantic Output base class creates a hybrid that: 1. Generates a dataclass __init__ that doesn't honor default_factory when the field is wrapped in Annotated[..., MaxItems(20)]. 2. Skips Pydantic's __pydantic_fields_set__ initialisation so attribute access via the instance breaks (.passed, .checks, .message). Plain Pydantic subclasses (the rest of the SDK's convention — see test_service.py's _RoutingInput/_RoutingOutput) work correctly. Removed the @DataClass decorator from both PreflightInput and PreflightOutput and added a comment explaining why future readers shouldn't add it back. Tests added at tests/unit/app/test_preflight.py — 8 cases covering: - PreflightOutput construction with/without checks kwarg (regression guard for the missing-default bug) - run_publish_preflight skip paths (user_id absent, empty user_id, service token unavailable) — these crashed at runtime previously - run_publish_preflight happy path (all checks pass, message set) - run_publish_preflight failure path (raises AppPermissionDeniedError with the first failing check's message)
…re is absent (HYP-829)
Live diagnosis from a deexp13p01 cron run showed:
[ERROR] application_sdk.infrastructure.secrets - Failed to fetch
deployment config key: ATLAN_AUTH_CLIENT_ID
[WARNING] application_sdk.app.preflight - run_publish_preflight:
service token unavailable — skipping checks.
Even though ATLAN_AUTH_CLIENT_ID was set on the pod via envFrom from the
argo-client-creds Kubernetes secret. Root cause: get_deployment_secret()
routes through Dapr to the 'deployment-secret-store' component, but app
worker pods only mount objectstore.yaml + secretstore.yaml — no
deployment-secret-store. Dapr returns no result, the SDK returns None,
the preflight skips.
atlan-publish-app (the only other app using these creds today) sidesteps
this by reading the env vars directly via os.getenv. Matching that
pattern: try Dapr first, fall back to os.environ.get for both the
service-token creds and the impersonation creds. No infra change needed;
unblocks the gate as soon as the env vars are wired via envFrom.
…ght-error-codes # Conflicts: # application_sdk/app/__init__.py # application_sdk/handler/service.py
…-829) Pre-commit hooks on CI flagged formatting drift in code touched by this PR's preflight + handler endpoint changes. Local pre-commit run produced: - ruff-format reformatted multi-arg HTTPException calls - isort consolidated import groups - PLC0415 caught 3 method-local imports without noqa annotations The local imports are intentional cold-path deferrals (matching the existing pattern in this file) — added 'noqa: PLC0415 — cold path' inline comments so ruff doesn't re-flag them.
Adds PublishPreflightMixin (from application_sdk.app) and check_user_enabled / check_atlan_publish_permission (from application_sdk.handler) to the auto-generated capability manifest. Generated by .claude/skills/capability-manifest extractor.
…rror (HYP-829)
CI surfaced TestHandlerError / test_back_compat failures:
AttributeError: property 'http_status' of 'HandlerError' object has no setter
Root cause: this branch added `http_status` as a read-only property on
the AppError base class. HandlerError (legacy AppError subclass) sets
`self.http_status = http_status` as an instance attribute in __init__.
The read-only property on the parent shadows that setter, so every
HandlerError construction now raises.
Main solved the same problem differently — a local `_CATEGORY_TO_HTTP`
dict + `_app_error_to_http_status(exc)` helper in handler/service.py
that doesn't touch the AppError class. Aligning with main:
- Drop the AppError.http_status property.
- Keep FailureCategory.http_status (no instance-setter conflict; it's
a property on the enum itself).
- Rewrite EC2 tests to read `error.category.http_status` instead of
`error.http_status` — same functional check, different access path.
Also includes a ruff-format pass on application_sdk/handler/checks.py
that was missed in the earlier commit.
…tegorical (HYP-829) Pre-commit's pinned ruff (different version than locally installed) prefers the tuple-arg form. One-line change in a single assert.
Removes the HYP-1309 semantic-status-code changes (not required for now): - errors/categories.py: FailureCategory.http_status property - handler/service.py: 422 body-validation guards on /auth /check /metadata - corresponding tests (test_categorical.py, test_service.py) Retains the HYP-1308 user gate: PublishPreflightMixin.run_publish_preflight + check_user_enabled / check_atlan_publish_permission helpers and their tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wires run_publish_preflight into SqlApp.run() as the first awaited step
(Phase 0) before extraction — covers scheduled runs that bypass Heracles.
SqlApp now inherits PublishPreflightMixin so the activity is registered.
The check is consolidated server-side: run_publish_preflight makes a single
call to Heracles' new /workflows/preflight/user-publish-check and raises
AppPermissionDeniedError on a failed verdict (fail-open on transport error).
checks.py is reduced to a thin client — the raw-httpx user lookup,
Keycloak token-exchange, and /evaluates plumbing are removed.
ExtractionInput gains user_id with AliasChoices("user_id","user-id") +
populate_by_name so Heracles' kebab-case user-id DAG arg actually populates
the field (the base Input validator only warned on it before, dropping it).
The unknown-key validator now also recognises validation aliases so it no
longer falsely warns "silently dropped" for aliased fields.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
…ate-only # Conflicts: # docs/agents/sdk-capabilities.md
Contributor
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 77.6%) Detailed Coverage Report |
Contributor
📚 Capability manifest drift detected
To resolve, pick one:
This check is informational, not blocking. Reviewers should ensure the manifest is |
Contributor
📦 Trivy Vulnerability Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailsrequirements.txtuv.lock |
Contributor
📦 Trivy Secret Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailsrequirements.txtuv.lock |
Collaborator
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
Contributor
|
🛠 Full Test Coverage Report: https://k.atlan.dev/coverage/application-sdk/pr/1966 |
rupeshatlan
requested review from
Aryamanz29,
OnkarVO7,
atlan-ci,
cmgrote,
louisnow and
vaibhavatlan
as code owners
cmgrote
requested changes
Jun 5, 2026
Collaborator
cmgrote
left a comment
cmgrote
left a comment
There was a problem hiding this comment.
New Heracles endpoints should be exposed via pyatlan, and then the existing pyatlan bits (client(s), etc) used for the actual interactions — token exchange, underlying Heracles API calls, etc.
Also, at the moment the PR solely supports SqlApps, a minority of the overall app landscape. (Not only the template, but the mixin itself is based on ExtractionInput, which is specific to SqlApps.)
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
4 participants
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.
Summary
Part of HYP-829 (Preflight 1 — user publish checks). Companion to heracles PR (head
ring-rupesh-hyp-829-user-gate-only).Wires
run_publish_preflightintoSqlApp.run()as the first awaited step (Phase 0), before extraction — so scheduled runs that bypass Heracles are still gated.SqlAppnow inheritsPublishPreflightMixin, so the activity is registered automatically for every SQL app.The check is consolidated server-side:
run_publish_preflightmakes a single call to Heracles' new/workflows/preflight/user-publish-checkand raisesAppPermissionDeniedError(non-retryable) on a failed verdict; fail-open on transport error.checks.pyis reduced to a thin client — the raw-httpx user lookup, Keycloak token-exchange, and/evaluatesplumbing are removed (addresses the earlier "reinventing pyatlan" review).ExtractionInputgainsuser_idwithAliasChoices("user_id","user-id")+populate_by_nameso Heracles' kebab-caseuser-idDAG arg actually populates the field — previously the baseInputvalidator only warned and Pydantic dropped it (the activity would have always self-skipped). The unknown-key validator now also recognises validation aliases so it no longer falsely warns "silently dropped".This branch is the user-gate-only line — the HYP-1309 semantic-status-code work was intentionally dropped per product call.
Tests
tests/unit/handler/test_checks.py(thin client: payload/headers/URL, parsing, non-200) andtests/unit/app/test_preflight.py(skip paths, pass, raise-on-fail, fail-open).Pending / reviewer notes
deexp(manual + scheduled, disabled-user T3) still TODO.docs/agents/sdk-capabilities.mdreferences the removedcheck_*helpers — regenerate via the skill (auto-generated, not hand-edited here).🤖 Generated with Claude Code