Remove legacy code - #7585
Draft
agoscinski wants to merge 5 commits into
Draft
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7585 +/- ##
==========================================
- Coverage 80.69% 80.67% -0.02%
==========================================
Files 581 582 +1
Lines 47139 46915 -224
==========================================
- Hits 38034 37842 -192
+ Misses 9105 9073 -32 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Both classes subclassed the deprecated `Code` for history rather than for behaviour: of its 36 methods they used only `__init__`, `_validate` and, for `PortableCode`, a `get_executable` that they already override. Both already called `super(Code, self)._validate()` to skip it, and both set `_EMIT_CODE_DEPRECATION_WARNING = False` purely to silence `Code.__init__`. Deriving them from `AbstractCode` directly drops all of that, along with the `_SKIP_MODEL_INHERITANCE_CHECK` opt-out their nested models needed while `Code` sat in the MRO. `CodeEntityLoader` and `execmanager` queried on `Code` to resolve any code, so they move to `AbstractCode`; `QueryBuilder` already widens an `AbstractCode` filter to the shared `data.core.code` prefix, so `load_code` keeps finding every code plugin. The two `get_execname` tests are removed rather than adapted: the method is part of the deprecated `Code` API and is not inherited any more.
Production storages contain nodes with `node_type = 'data.core.code.Code.'`. Once the `core.code` entry point is gone, `load_node_class` would fall back to `Data` for them without a word, so the codes would quietly lose their entire API while still looking fine. A stored legacy code already carries everything a modern one needs, so convert it at the storage layer, where the data is, rather than at load time: - `is_local=False` -> `InstalledCode`, `remote_exec_path` -> `filepath_executable` - `is_local=True` -> `PortableCode`, `local_executable` -> `filepath_executable` Every other attribute uses the same key on `AbstractCode` and is left alone, as are the repository contents a portable code needs. The node type and one attribute key change, so `_aiida_hash` no longer describes the node and is dropped; `verdi node rehash` recomputes it. The same rewrite lands in all three storages: `psql_dos` and `sqlite_dos` as `main_0003`, and the archive format as `main_0002`. Static archives that were at the previous archive version are migrated along with it, two of which turn out to contain exactly such a legacy code.
Deprecated since `aiida-core==2.1` and scheduled for removal in `3.0`. The class and its `core.code` entry point are gone, along with the ~36 deprecated methods it carried. Everything that used `Code` to mean "any code" now uses `AbstractCode`: the `verdi code` queries, the REST translator, the dumping filters and the `verdi shell` auto-imports (which gain `InstalledCode` and `PortableCode`). The graph visualisation special-cased the legacy node type for both its style and its sublabel; it now covers the three concrete code types instead. `load_node_class` raises `IncompatibleStorageSchema` for the legacy type string rather than falling back to `Data`, so a storage that was never migrated says so instead of silently degrading. The conversion itself belongs to the storage migration and is not attempted here. The archive importer's special case for the legacy type is dropped: it stripped the `hidden` extra, which is a legitimate extra on the modern code plugins and is preserved for them already. `_KEY_ATTRIBUTE_DEFAULT_CALC_JOB_PLUGIN` and `_KEY_EXTRA_IS_HIDDEN` keep the stored names they inherited from `Code`. Renaming them needs its own attribute migration of every stored code, so that is left for a separate change and the comments now say so.
agoscinski
force-pushed
the
remove-legacy-code
branch
from
August 31, 2026 19:39
10726b2 to
4b2a77a
Compare
The migration test files for the `psql_dos` and `sqlite_dos` backends share the basename `test_main_0003_migrate_legacy_code.py`. Since neither directory was a package, pytest resolved both to the same module name and failed to collect the second one with an import file mismatch. Add the missing `__init__.py` files, as is already done for the sibling `psql_dos/migrations/django_branch` directory.
agoscinski
force-pushed
the
remove-legacy-code
branch
from
August 31, 2026 19:44
4b2a77a to
5d57f06
Compare
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
1 participant
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.
This simplifies the redesign of the ORM, because some parts of the fields logic is only required because of some renaming while keeping backwards compatibility. While this logic could be used in the future when a change with backwards compatibility is needed, I would rather not have this extra logic and wait for another major release to break things instead of having this backwards compatibility layer making the logic more complicated.