Skip to content

Engine : RemoteStashFolderData could be refactored to RemoteStashCopiedData #6784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
khsrali opened this issue Mar 5, 2025 · 2 comments · May be fixed by #6825
Open

Engine : RemoteStashFolderData could be refactored to RemoteStashCopiedData #6784

khsrali opened this issue Mar 5, 2025 · 2 comments · May be fixed by #6825
Assignees
@khsrali

Comments

@khsrali
Copy link
Contributor

khsrali commented Mar 5, 2025

While changes for stashing are upcoming, specifically this PR #6746
It makes more sense to refactor RemoteStashFolderData to RemoteStashCopiedData and deprecate the other one.

The new class will also accommodate a new property, which is uuid of the original calcjob.
More description is given in this comment by @agoscinski .
@khsrali khsrali self-assigned this Mar 5, 2025
@agoscinski
Copy link
Contributor

agoscinski commented Mar 26, 2025 •
edited
Loading

We need to rediscuss this a bit because the class RemoteStashFolderData is not only specifying the stash mode but also offering a dedicated API for folders. In RemoteData we have the problem that this API is not in a separate class (like inRemoteStashData and RemoteStashFolderData) but all mixed in it. The reason was that RemoteData is documented to be used only for folders (even though this is also not consistent if one looks at the docstring). So we need to find a solution that is consistent with changes we would like to do in RemoteData.

Linking relevant issues and PRs for RemoteData #6807 #6804
Linking previous discussion on RemoteStashFolderData for reference #6746 (comment)
Ping @GeigerJ2

@GeigerJ2 GeigerJ2 mentioned this issue Mar 27, 2025
Draft
@agoscinski
Copy link
Contributor

Discussed with @khsrali in case of the stashing the distinction between folder and file does not make sense since the targeted files/folders (source_list) are always put into a folder (target_base). The source_list can be a mix of files and folders. One can therefore find a solution to support single files with RemoteData (#1609) separately from the design of the RemoteStash* classes.

@khsrali khsrali linked a pull request Apr 16, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@khsrali khsrali

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Deprecate RemoteStashFolderData and introduce an additional property source_uuid khsrali/aiida-core
2 participants
@agoscinski @khsrali