Add support for export and import commands #593
Conversation
Riushda
force-pushed
the
issue/add_export_and_import_commands
branch
from
77b7b17 to
6755b86
Compare
inknos
left a comment
inknos
left a comment
There was a problem hiding this comment.
Overall looks good, only two points to address.
Minor fix, based on my preference: would you consider squashing the commits together? I believe a single commit would fit nicely in the log for this change.
thanks!
Riushda
force-pushed
the
issue/add_export_and_import_commands
branch
3 times, most recently
from
4642c37 to
e8e42e7
Compare
Riushda
force-pushed
the
issue/add_export_and_import_commands
branch
from
e8e42e7 to
7f3b50a
Compare
|
code looks good now! let me be pedantic, but could you add a test for |
Signed-off-by: Riushda <mohandesdariush@gmail.com> fix export and import functions Signed-off-by: Riushda <mohandesdariush@gmail.com> [docs] Update testing section Add common issues section Add docs on how to skip tests based on podman's version and os Signed-off-by: Nicola Sella <nsella@redhat.com> Signed-off-by: Riushda <mohandesdariush@gmail.com> format Signed-off-by: Riushda <mohandesdariush@gmail.com> Add import and export test Signed-off-by: Riushda <mohandesdariush@gmail.com> Do not test if podman version is older than 5.6.0 Signed-off-by: Riushda <mohandesdariush@gmail.com> Fix docstring Signed-off-by: Riushda <mohandesdariush@gmail.com> Fix export archive command Signed-off-by: Riushda <mohandesdariush@gmail.com> Add path to import function Signed-off-by: Riushda <mohandesdariush@gmail.com> Add default values Signed-off-by: Riushda <mohandesdariush@gmail.com> Fix typing Signed-off-by: Riushda <mohandesdariush@gmail.com> Add mock tests Signed-off-by: Riushda <mohandesdariush@gmail.com>
Riushda
force-pushed
the
issue/add_export_and_import_commands
branch
from
7f3b50a to
382d30a
Compare
| content=b'exported_archive', | ||
| ) | ||
| actual = self.client.volumes.export_archive("dbase") | ||
| self.assertIsInstance(actual, bytes) |
There was a problem hiding this comment.
To be honest, I'm not sure about the value of these mock tests (for the import and export). I wouldn't have added them if you didn't ask for it.
There was a problem hiding this comment.
My motivation is primarily code coverage. In addition to this, tests help me a lot with future features working with the community, even when they are simple.
I'd like to hear your opinion: why you are not sure of the value for these specific tests. Do you mean they are too trivial?
thanks a lot for the patience and for updating the pr
There was a problem hiding this comment.
I was mainly talking about the functions I added. There isn't much to test without an actual volume and archive file. But don't get me wrong, I think mock tests in general are great to add more test coverage in particular for complex scenario that are not easy to reproduce in actual integration test.
There was a problem hiding this comment.
I think it would be very nice to do something like this in python, but we are missing other options in the python bindings to do so
podman volume create testvol
podman volume export testvol --output testvol.tar
mkdir testvol
tar -xvf testvol.tar -C testvol/
touch testvol/test
tar -rf testvol.tar testvol/test
cat testvol.tar | podman volume import testvol -
podman unshare
podman volume mount testvol
ls /home/nsella/.local/share/containers/storage/volumes/testvol/_data/testvol/testSo I think your tests are now good
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: inknos, Riushda The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Thanks for the review ! |
2 participants
Podman 5.6.0 introduces 2 new commands: podman volume import and export. I was willing to add support for these 2 commands in the python sdk because they are very useful in some specific volumes backup scenario I have in mind.
Closes #574