feat(contract-toolkit): codegen _e2e_base.py, _e2e_credential.py, _e2e_substitutions.py#1980
Open
cmgrote wants to merge 8 commits into
Open
feat(contract-toolkit): codegen _e2e_base.py, _e2e_credential.py, _e2e_substitutions.py#1980cmgrote wants to merge 8 commits into
cmgrote wants to merge 8 commits into
Conversation
…e_substitutions.py Adds three new generator functions to App.pkl so that `pkl eval -m . contract/app.pkl` now produces the e2e test harness files that were previously hand-authored (and had already drifted — mysql's _e2e_credential.py silently dropped iam_user/iam_role fields). Changes: - App.pkl: new contract fields `argoPackageName`, `argoTemplateName`, `appServiceUrl` (defaults derived from `name`; 95% of connectors never override) - App.pkl: `generateE2EBasePy()` — always emits `_e2e_base.py` with required harness attrs; routes to SQLAppE2ETest for warehouse/database/lake/queryengine connectors - App.pkl: `generateE2ECredentialPy()` — emits `_e2e_credential.py` when credential config is present; unions all auth options into one model (auth-specific fields get empty defaults); extraFields from the URL group become Optional fields - App.pkl: `generateE2ESubstitutionsPy()` — emits `_e2e_substitutions.py` when uiConfig has fields beyond the parent class; widget-type-aware typing (Literal[...] for bounded Radio/DropDown, bool for BooleanInput, int for NumericInput, dict[str,Any]|None for SqlTree/NestedInput) - scripts/test-sdk-import.py: extended to also import-test all generated _e2e_*.py files - testing/e2e/credential.py + substitutions.py: docstrings updated from aspirational (E2EOutput.pkl) to descriptive (generateE2ECredentialPy / generateE2ESubstitutionsPy) - examples/*/app/generated/_e2e_*.py: golden fixtures for all 7 examples
cmgrote
requested review from
Aryamanz29,
OnkarVO7,
atlan-ci,
fyzanshaik-atlan,
louisnow and
vaibhavatlan
as code owners
Contributor
|
Docs reminder:
Per toolkit convention, public PKL changes should update:
Ignore this reminder only when the source change is internal and has no author-facing impact. |
✅ 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. |
Contributor
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 77.6%) Detailed Coverage Report |
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 |
…ate-all.sh ruff Pre-commit ruff wrapped two long Literal Field lines to multi-line; the ruff bundled in regenerate-all.sh leaves them single-line. Commit the script's output so the stale-check in CI passes.
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/1980 |
Collaborator
🧪 Connector Tests — atlan-mysql-appStatus: Passed ✅
|
Collaborator
🧪 Connector Tests — atlan-openapi-appStatus: Passed ✅
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the contract-toolkit’s PKL-driven code generation to emit the Python e2e harness artifacts (_e2e_base.py, _e2e_credential.py, _e2e_substitutions.py) directly from contract/app.pkl, and updates the SDK docs + import-test script to match the new generation source of truth.
Changes:
- Adds E2E harness codegen functions + new overridable contract fields (
argoPackageName,argoTemplateName,appServiceUrl) incontract-toolkit/src/App.pkl. - Extends
scripts/test-sdk-import.pyto import-test all generated_e2e_*.pyfiles in addition to_input.py. - Updates SDK e2e docstrings and refreshes example generated e2e files.
Reviewed changes
Copilot reviewed 4 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| contract-toolkit/src/App.pkl | Adds new E2E harness contract fields and PKL codegen for _e2e_base.py, _e2e_credential.py, _e2e_substitutions.py. |
| contract-toolkit/scripts/test-sdk-import.py | Expands import validation to cover _e2e_*.py outputs. |
| contract-toolkit/examples/publish-controls/app/generated/_e2e_substitutions.py | Adds generated mustache substitutions model for the example. |
| contract-toolkit/examples/publish-controls/app/generated/_e2e_credential.py | Adds generated credential body model for the example. |
| contract-toolkit/examples/publish-controls/app/generated/_e2e_base.py | Adds generated e2e base harness class for the example. |
| contract-toolkit/examples/minimal/app/generated/_e2e_substitutions.py | Adds generated mustache substitutions model for the example. |
| contract-toolkit/examples/minimal/app/generated/_e2e_base.py | Adds generated e2e base harness class for the example. |
| contract-toolkit/examples/full/app/generated/_e2e_substitutions.py | Adds generated mustache substitutions model for the example (incl. Literal/Any typing). |
| contract-toolkit/examples/full/app/generated/_e2e_credential.py | Adds generated credential body model for the example (incl. optional fields). |
| contract-toolkit/examples/full/app/generated/_e2e_base.py | Adds generated e2e base harness class for the example. |
| contract-toolkit/examples/fanin/app/generated/_e2e_credential.py | Adds generated credential body model for the example. |
| contract-toolkit/examples/fanin/app/generated/_e2e_base.py | Adds generated e2e base harness class for the example. |
| contract-toolkit/examples/deploy/app/generated/_e2e_substitutions.py | Adds generated mustache substitutions model for the example. |
| contract-toolkit/examples/deploy/app/generated/_e2e_base.py | Adds generated e2e base harness class for the example. |
| contract-toolkit/examples/connection-ref/app/generated/_e2e_base.py | Adds generated e2e base harness class for the example. |
| contract-toolkit/examples/bundle/app/generated/miner/_e2e_substitutions.py | Adds generated mustache substitutions model for the miner example. |
| contract-toolkit/examples/bundle/app/generated/miner/_e2e_base.py | Adds generated e2e base harness class for the miner example. |
| contract-toolkit/examples/bundle/app/generated/crawler/_e2e_credential.py | Adds generated credential body model for the crawler example. |
| contract-toolkit/examples/bundle/app/generated/crawler/_e2e_base.py | Adds generated e2e base harness class for the crawler example. |
| application_sdk/testing/e2e/substitutions.py | Updates module docstring to point at the new generator function in App.pkl. |
| application_sdk/testing/e2e/credential.py | Updates module docstring to point at the new generator function in App.pkl. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fmtSubsField helper wraps Field() calls that exceed 88 chars (matching ruff's default), so regenerate-all.sh and pre-commit ruff-format always agree on output.
- App.pkl: auth_type default now uses credentialAuthDefault when set, falls back to first credentialAuthOptions key (matches credential config generation at L1890) - test-sdk-import.py: remove unused _E2E_CLASSES dict - testing/e2e/credential.py: expand docstring to accurately list all field sources (credentialUrlGroup, credentialSharedExtraFields, credentialCommonFields, auth option fields, auth option extraFields)
Collaborator
Author
|
@sdk-review |
Contributor
Earlier @sdk-review trigger (click to expand)🔍 SDK Review (mothership) triggered by @cmgrote at 2026-06-03T12:25:53.743Z. Watch the workflow run live — the review summary will appear as a separate comment when complete (typical: 5–30 min, hard cap 2h). 🟥 Run ended without a clean completion — status |
Collaborator
Author
|
@sdk-review |
Contributor
Earlier @sdk-review trigger (click to expand)🔍 SDK Review (mothership) triggered by @cmgrote at 2026-06-03T12:52:19.083Z. Watch the workflow run live — the review summary will appear as a separate comment when complete (typical: 5–30 min, hard cap 2h). ✅ Completed — status |
Contributor
SDK Review (mothership): PR #1980 — feat(contract-toolkit): codegen _e2e_base.py, _e2e_credential.py, _e2e_substitutions.pyVerdict: NEEDS FIXES
PR Title Check
Findings
Holistic Recommendations
Strengths
CI: all passing | check via |
![mothership-ai[bot]](https://avatars.githubusercontent.com/in/2372052?s=60&v=4){kind=small}
Signed-off-by: Chris (He/Him) <cgrote@gmail.com>
- App.pkl: add escapePyStr() helper; use it for all string class attrs in _e2e_base.py codegen so quote/backslash/newline chars in overrides produce valid Python rather than broken literals - App.pkl: widen pascalCase splitter from [-_]+ to [^A-Za-z0-9]+ so dots, slashes, and other non-alphanumeric chars are handled consistently - App.pkl: add frozen=True to generated CredentialBodyExtra ConfigDict, matching the immutability of the CredentialBody base class - credential.py: fix docstring — auth-specific fields use sentinel defaults (str='', int=0), not Optional; conditional common fields become Optional - test-sdk-import.py: test_e2e() now verifies expected class suffix exists and checks harness attrs for _e2e_base / model_fields for credential+subs - docs/reference.md: add E2E Test Harness subsection documenting the three new overridable fields (argoPackageName, argoTemplateName, appServiceUrl)
47 test cases covering generateE2EBasePy, generateE2ESubstitutionsPy, generateE2ECredentialPy, pascalCase, fmtSubsField, and getSubsFieldLine. Also moves _e2e_*.py output outside the uiConfig!=null guard so the test harness files are emitted in all single-entrypoint contracts regardless of whether uiConfig is set.
Collaborator
Author
|
@sdk-review |
Contributor
Earlier @sdk-review trigger (click to expand)🔍 SDK Review (mothership) triggered by @cmgrote at 2026-06-03T13:53:26.635Z. Watch the workflow run live — the review summary will appear as a separate comment when complete (typical: 5–30 min, hard cap 2h). 🟥 Run ended without a clean completion — status |
Contributor
SDK Re-review (mothership): PR #1980 — feat(contract-toolkit): codegen _e2e_base.py, _e2e_credential.py, _e2e_substitutions.pyVerdict: READY TO MERGE
PR Title Check
Affected Toolkit Surfaces
Cross-Repo Validation
Delta from previous review
Findings
Holistic Recommendations
Strengths
CI: all required checks passing (sdk-review pending = this run) | check via |
atlan-ci
previously approved these changes
Jun 3, 2026
Collaborator
atlan-ci
left a comment
atlan-ci
left a comment
There was a problem hiding this comment.
SDK reviewer's verdict: READY TO MERGE.
Full review summary is in the comment posted on this PR.
…ons docstring Points to generateE2ESubstitutionsPy() in App.pkl, matching the correction already applied to the file-level docstring.
Collaborator
Author
|
@sdk-review |
Contributor
|
🔍 SDK Review (mothership) triggered by @cmgrote at 2026-06-03T15:00:37.893Z. Watch the workflow run live — the review summary will appear as a separate comment when complete (typical: 5–30 min, hard cap 2h). ✅ Completed — status |
Contributor
Contract Toolkit Re-review (mothership): PR #1980 — feat(contract-toolkit): codegen _e2e_base.py, _e2e_credential.py, _e2e_substitutions.pyVerdict: READY TO MERGE
PR Title Check
Affected Toolkit Surfaces
Cross-Repo Validation
Delta from previous review
FindingsNo findings. Holistic Recommendations
Strengths
CI: all required checks passing (sdk-review pending = this run) |
atlan-ci
approved these changes
Jun 3, 2026
Collaborator
atlan-ci
left a comment
atlan-ci
left a comment
There was a problem hiding this comment.
SDK reviewer's verdict: READY TO MERGE.
Full review summary is in the comment posted on this PR.
Contributor
Contract Toolkit Re-review (mothership): PR #1980 — feat(contract-toolkit): codegen _e2e_base.py, _e2e_credential.py, _e2e_substitutions.pyVerdict: READY TO MERGE
PR Title Check
Affected Toolkit Surfaces
Cross-Repo Validation
Delta from previous review
FindingsNo findings. The single-commit delta is a docstring correction that resolves the only outstanding nit; no new code, no contract changes, no public-API surface touched. Holistic Recommendations
Strengths
CI: all required checks passing (sdk-review pending = this run) |
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.
Summary
App.pklsopkl eval -m . contract/app.pklnow produces the e2e test harness files that were previously hand-authored (with misleading "Generated from contract/app.pkl" banners that weren't true)argoPackageName,argoTemplateName,appServiceUrl— with defaults derived fromname; 95% of connectors never overridescripts/test-sdk-import.pyto import-test all generated_e2e_*.pyfiles (was only testing_input.py)testing/e2e/credential.pyandtesting/e2e/substitutions.pyto reference the actual codegen functions instead of the never-createdE2EOutput.pklWhat's generated
_e2e_base.py_e2e_credential.pyhasCredentialConfig = trueand at least one auth option defined_e2e_substitutions.pyuiConfighas fields beyond what the parent class already providesBase class routing: warehouse/database/lake/queryengine connectors →
SQLAppE2ETest/SQLMustacheSubstitutions; others →BaseE2ETest/MustacheSubstitutions.Widget-type-aware substitution typing:
Literal[...]for bounded Radio/DropDown,boolfor BooleanInput,intfor NumericInput,dict[str, Any] | Nonefor SqlTree/NestedInput — soimport_type="HTTP"against the OpenAPI connector (valid values:"URL","CLOUD") fails at type-check time, not at runtime.Credential body: all auth options unioned into one model; auth-specific-only fields get empty-string defaults;
ConditionalFieldSpecs becomestr | None.Verification
Follow-up (separate PRs in consumer repos, after toolkit version bump)
atlan-openapi-app: regenerate to replaceAtlanConnectorType.API.valueindirection withconnection_type = "api"atlan-mysql-app: regenerate to gainiam_user/iam_roleunion'd fields and the missing banner header🤖 Generated with Claude Code