-
Notifications
You must be signed in to change notification settings - Fork 3
[ACIX-1225] Create dda env dev fs import and export commands
#233
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.
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
[ACIX-1225] Create dda env dev fs import and export commands
#233
Conversation
bffa189 to
b055041
Compare
b055041 to
dbe0c30
Compare
dbe0c30 to
95901d4
Compare
…ort_from_dir` in dev envs
dda env dev fs import and export commands
eb939cb to
721dada
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements dda env dev fs import and dda env dev fs export commands that enable easy file transfer between the host filesystem and development environments. The feature is designed to be extensible to different environment types (not just Linux containers) by implementing the functionality through the dev env interface.
Key Changes:
- Added abstract
export_filesandimport_filesmethods to the dev environment interface - Implemented core file operation logic in
src/dda/env/dev/fs.pythat's agnostic to environment type - Created CLI commands for export, import, and an internal localimport command for use inside containers
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/dda/env/dev/interface.py |
Added abstract methods export_files and import_files to the dev environment interface |
src/dda/env/dev/fs.py |
New module containing environment-agnostic file operation logic (import_from_dir, determine_final_copy_target, handle_overwrite) |
src/dda/env/dev/types/linux_container.py |
Implemented export_files and import_files methods for Linux containers using docker cp and shared directories; added per-instance shared directory mount |
src/dda/utils/fs.py |
Enhanced temp_directory function to accept optional dir parameter for controlling where temporary directories are created |
src/dda/cli/env/dev/fs/__init__.py |
New CLI group for filesystem operations |
src/dda/cli/env/dev/fs/export/__init__.py |
CLI command for exporting files from dev environments to host |
src/dda/cli/env/dev/fs/import/__init__.py |
CLI command for importing files from host to dev environments |
src/dda/cli/env/dev/fs/localimport/__init__.py |
Internal CLI command (hidden) used inside dev environments to perform the actual file import |
tests/env/dev/test_fs.py |
Comprehensive unit tests for the file operation logic in fs.py |
tests/env/dev/test_interface.py |
Updated interface test stub to include new methods |
tests/env/dev/types/test_linux_container.py |
Updated tests for shared directory path changes and added tests for export/import orchestration |
tests/env/dev/fixtures/fs_tests/* |
Test fixture files for verifying file operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ofek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
…inuxContainer.export_files`
Revert "fix(fs): Make sure we `docker cp` into the temporary directory"
This reverts commit 82d700a.
# This is the commit message #2:
Revert "feat(env): Implement `LinuxContainer.import_files`"
This reverts commit d12ba33.
# This is the commit message #3:
Revert "feat(env): Implement hidden `localimport` command to easily call `import_from_dir` in dev envs"
This reverts commit 08d4b2e.
# This is the commit message #4:
Revert "refactor(env): Move host filesystem import logic to `fs.py`"
This reverts commit 95901d4.
# This is the commit message #5:
Revert "feat(env): Implement `LinuxContainer.export_files`"
This reverts commit 3cd4d22.
- Only accept a single source argument - `--force` will be pretty much always implied (depending on the semantics of `cp/mv`) - `--recursive` is always implied - We'll reimplement `mkpath` if a customer asks for it
aed5786 to
50a9153
Compare
50a9153 to
b392a7c
Compare
ofek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥇
* feat(env): Create `fs` command group with `export` and `import` command stubs
* feat(fs): Implement skeleton for `export` and `import` commands
* fix(fs): Pass non-host paths as `str`
* feat(env): Implement `LinuxContainer.export_files`
* refactor(env): Move host filesystem import logic to `fs.py`
* feat(env): Implement hidden `localimport` command to easily call `import_from_dir` in dev envs
* feat(fs): Allow passing `dir` to `temp_directory`
* feat(env): Implement `LinuxContainer.import_files`
* fix(linux_container): Actually mount the shared directory into the container
* fix(env): Small fixes for `import` command
* fix(fs): Make sure we `docker cp` into the temporary directory
* refactor: Copilot-suggested review changes
* Revert initial implementation of `LinuxContainer.export_files` and `LinuxContainer.export_files`
Revert "fix(fs): Make sure we `docker cp` into the temporary directory"
This reverts commit 82d700a.
# This is the commit message #2:
Revert "feat(env): Implement `LinuxContainer.import_files`"
This reverts commit d12ba33.
# This is the commit message #3:
Revert "feat(env): Implement hidden `localimport` command to easily call `import_from_dir` in dev envs"
This reverts commit 08d4b2e.
# This is the commit message #4:
Revert "refactor(env): Move host filesystem import logic to `fs.py`"
This reverts commit 95901d4.
# This is the commit message #5:
Revert "feat(env): Implement `LinuxContainer.export_files`"
This reverts commit 3cd4d22.
* refactor(fs): Simplify interface for `import` and `export` commands
- Only accept a single source argument
- `--force` will be pretty much always implied (depending on the semantics of `cp/mv`)
- `--recursive` is always implied
- We'll reimplement `mkpath` if a customer asks for it
* feat(fs): Implement `LinuxContainer.export_path`
* feat(fs): Create `dda.utils.fs::cp_r` function for matching `cp -r` behavior with `shutil` methods
* feat(fs): Implement `LinuxContainer.import_path`
* fix: Update copyright years
* fix(docs): Misnamed parameter
* fix(tests): Fix for windows paths 68af61f
Create two commands,
dda env dev fs importanddda env dev fs exportthat allow for easily moving files and directories in and out of dev envs.This is useful as an addition to standard bind mounts, because people might not think of bind-mounting something into the container upon creation. Such a feature was heavily requested by our users.
The feature was implemented by making the necessary changes to the dev env interface to make sure that such a feature will also be supported by other dev env types (remote VMs...). As much logic as possible was made agnostic to the type of dev env in question (everything in
src/dda/env/dev/fs.py)