Repository navigation
Conversation
…he import path Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 75.0%, actual: 80.0%) Detailed Coverage Report |
Contributor
☂️ Code Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
…ributes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
OnkarVO7
marked this pull request as ready for review
November 10, 2026 07:23
OnkarVO7
requested review from
Aryamanz29,
atlan-ci,
cmgrote and
vaibhavatlan
as code owners
November 10, 2026 07:23
OnkarVO7
marked this pull request as draft
November 10, 2026 07:32
OnkarVO7
marked this pull request as ready for review
November 10, 2026 07:59
This branch has not been deployed
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.
Linear: AUT-1240
What
application_sdk/validation/__init__.pyimportsvalidation.assetsandvalidation.asset_rulesat module scope. Both importpyatlan_v9at module scope, which cascades intopyatlan. So importing anything from this package loads the entire typed asset model — 238 + 695 modules, ~140 MiB resident.This serves those 13 names lazily via the package's existing
lazy_getattrhelper (PEP 562), the same pattern already used byhandler,clients,contracts,credentialsand five other packages here.__all__is unchanged and every name still resolves; aTYPE_CHECKINGblock keeps the annotations intact for type checkers.Why it matters
The cost lands on every worker that registers activities.
create_activity_from_task(execution/_temporal/activities.py:341) importsvalidation.interceptor, and importing anyvalidationsubmodule executes the package__init__:interceptor.pyimports onlyvalidation.artifactsandvalidation.sources. It never touchesassets. The whole 140 MiB is a barrel pulling in a sibling that nobody on this path asked for.The import at
activities.py:341already carries a comment explaining it was pushed into function scope so the handler and server processes would not pay forpyatlan_v9. That fixed half the problem — the worker still pays, because for the worker the import does happen.Measured
Automation Engine worker, 2x2 factorial in a single venv on one machine, 3 reps per arm, median reported. The only SDK variable is this package's
__init__.py, swapped in place between the eager and lazy versions — the eager file is byte-identical to released 3.52, so there is no version drift between arms. "Full boot" = the worker's boot-path imports plusget_all_task_activities().This change is worth ~139 MiB on its own and is independent of the Automation Engine work: it removes the registry-scan cost (147.3 -> 0.6 MiB) and leaves boot-path imports untouched, while AE #1364 does the reverse. The two compose to -259 MiB without overlapping.
get_all_task_activities()still returns the same 40 activities.This is deferral, not removal: a caller that actually validates assets pays the same cost at that point. Most activity workers never do.
The
main / noarm is the noisy one (393.8 / 401.0 / 414.1 across reps, ~+-10 MiB); every other arm varied by under 0.5 MiB.One real behaviour change, and how it is handled
The eager imports had an incidental side effect:
from .assets import Xbindsassetsas an attribute of the package. Dropping them would make this start raisingAttributeError:This is handled by passing
lazy_getattr's existingsubmodulesargument:Those two submodules are imported on access, so the attribute works and the dependency floor is unchanged — a bare
import application_sdk.validationstill loads nopyatlan.Worth flagging how this was nearly missed: the first compatibility probe reported
v.assetsas fine. It was not. An earlier probe in the same process had imported the submodule, and importingpkg.subanywhere bindssubonpkg— so the assertion passed for the wrong reason. Re-run in a fresh interpreter it failed. The regression test for it is subprocess-based for exactly this reason.Consumer impact
Swept the org for every repo that touches this package (5 queries, not 1 — searching the dotted path alone misses bare-symbol imports and
from application_sdk import validation). Seven non-SDK repos reference it. Every real import is one of two patterns, both unaffected:atlan-hive-app,atlan-teradata-appfrom application_sdk.validation import validate_transformed_dir__getattr__atlan-workdayprismanalytics-appfrom application_sdk.validation.assets import validate_assetatlan-sapanalyticscloud-appfrom application_sdk.validation.ndjson import iter_ndjson_linesThe remaining references (
atlan-cosmosdb-app,atlan-trino-app,atlan-coalesce-app,atlan-quicksight-app,connector-pulse,marketplace-partner) are docstrings, READMEs, ADRs and log files, not code.from application_sdk import validationhas zero hits org-wide. Inside this repo, everyvalidation.assetsreference is a Sphinx:func:/:mod:docstring link, not attribute access.Caveat: code search only indexes default branches, so it cannot see feature branches or unindexed repos. That is why the
_SUBMODULESguard is in rather than relying on the sweep — an unseen consumer using the attribute form keeps working regardless.This closes a gap the package already documents
The module docstring states a standing dependency floor — nothing imports
pyarrow,pandasorpanderaat module scope — andtest_artifact_dependency_floor.pyenforces it statically.pyatlan_v9is heavier than all three and was not covered.ndjson.py's docstring said its deferred import ofassets"buys no dependency-floor benefit — this package's__init__importsassetsat module scope, sopyatlan_v9is already loaded by the time anything can import this module." This change makes that false, so it is corrected: the deferral does now buy the benefit.Tests
Three tests added to
test_artifact_dependency_floor.py. Each was verified to fail without its corresponding fix, so none is vacuous:test_importing_the_package_does_not_load_pyatlan— subprocess, because whetherpyatlan_v9is warm in the test interpreter depends on what an earlier test imported. Fails against the eager barrel (True != False).test_every_lazy_name_still_resolves— every__all__entry is reachable, so deferral cannot silently drop a name.test_the_asset_submodules_are_still_package_attributes— subprocess, loops over("assets", "asset_rules"). Fails without_SUBMODULES.uv run pytest tests/uniton the merged branch — 14,323 passed, 12 skipped, 5 deselected, 0 failed, 0 errors. (The 3 failures and 10 errors noted on earlier revisions of this PR were pre-existing onmainand have since been fixed there.)pre-commit(ruff check, ruff format, pyright) clean.Scope
3 files, +140/-18: the barrel, the
ndjson.pydocstring the change made false, and the tests.Downstream
Opened as a draft pending review of the consumer side. Automation Engine's matching change is atlanhq/atlan-automation-engine-app#1364, which pins this branch for end-to-end testing. With both landed its worker boot drops 401 MiB -> 142 MiB.
🤖 Generated with Claude Code