Replace `RemoteStashFolderData` with `RemoteStashCopyData`
Fix: #6784
Deprecates the legacy stash node type RemoteStashFolderData and replace it with RemoteStashCopyData.
This is only for naming convention which we have discussed before with @agoscinski.
This PR also contains docstring cleanup for RemoteStashCompressedData
~This PR also introduces a new property source_uuid. This is useful for un-stashing, and re-stashing purposes in the coming changes.~
~Note the 1 & 2 are entirely different changes, and deprecation is NOT occurred for introducing source_uuid.~
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 79.06%. Comparing base (48991ab) to head (07b37d6).
:warning: Report is 83 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #6825 +/- ##
==========================================
+ Coverage 79.06% 79.06% +0.01%
==========================================
Files 565 566 +1
Lines 43126 43155 +29
==========================================
+ Hits 34095 34118 +23
- Misses 9031 9037 +6
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Just talked to @agoscinski ,
We don't actually need to store the uuid actually. We could just use the traversals to reach there:
from aiida.orm import load_node, CalcJobNode, Node
def traverse(node: Node) -> CalcJobNode | None:
for l in node.base.links.get_incoming():
if isinstance(l.node, CalcJobNode):
calcjob_node = l.node
return calcjob_node
return traverse(l.node)
return None
node = load_node(12)
calcjob_node = traverse(node)
if None:
raise ValueError("Your stash node is not connected to any calcjob node")
print(calcjob_node)
Therefore, this PR, has to be only do the deprecation job.
@agoscinski I applied your previous review applied, Please have a look on Monday, now it should be a quick review
I applied your review @agoscinski We can talk in person about QB
Also please just reply on
RemoteStashData should be actually an abstract class right? I mean it should never been used but only the child classes. Maybe we can do this in a subsequent PR?
Draft for commit, please check
`RemoteStashFolderData` has been so far only supported the `StashMode.COPY`
option and unlike the name suggests supported also to copy individual file(s).
To be more consistent with the naming scheme of `RemoteData` that also supports
files and folders and `RemoteStashCompressedData` we switch to the name
`RemoteStashCopyData`. For backwards compatibility reasons we keep
`RemoteStashFolderData` but disable the possibility of its construction and
introduce the new class `RemoteStashCopyData` that works exactly as
`RemoteStashFolderData`. This PR also contains docstring cleanup for
`RemoteStashCompressedData`.
We discussed that we need a migration for this as we do not want to have two datatypes in the database that basically do the same thing. Introducing the database migration in the future will causes problem as we need to resolve conflicts in the pk. No one in the team knows how the migration works and there is no guide that makes this straightforward. One can figure this out with the existing files and material but it will take a bit of time and this renaming is not essential so we postpone this PR from v2.7.0.
We discussed that we need a migration for this as we do not want to have two datatypes in the database that basically do the same thing. Introducing the database migration in the future will causes problem as we need to resolve conflicts in the
pk. No one in the team knows how the migration works and there is no guide that makes this straightforward. One can figure this out with the existing files and material but it will take a bit of time and this renaming is not essential so we postpone this PR fromv2.7.0.
yes, pretty much that's all to be said :man_shrugging:
I close this for now.
The implementation of this commit might get used in a more broad PR that deprecates all RemoteStash*Data by removing abstractification in RemoteStashData