-
Notifications
You must be signed in to change notification settings - Fork 192
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
Dependencies: Update to disk-objectstore~=1.0
#6132
Conversation
dfe844c
to
23173e4
Compare
2c788fd
to
f34f7f1
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.
I left a couple of comments - in particular good to double check the one on tests (tests seem to be the same but the test results changed?), then it can be merged (Note: I didn't check if there were other occurrences of data classes still used in the old dict way, but I imagine we can fix those later if we spot them)
|
||
|
||
#yapf: disable | ||
@pytest.mark.parametrize(('kwargs', 'output_info'), ( | ||
( | ||
{'live': True}, | ||
{'unpacked': 2, 'packed': 4} | ||
{'unpacked': 0, 'packed': 4} |
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.
Is it expected that these numbers changed?
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.
Yes, I believe this is because the live maintenance now automatically includes packing and cleaning
:param do_repack: flag for forcing the re-packing of already packed files. | ||
:param clean_storage: flag for forcing the cleaning of soft-deleted files from the repository. | ||
:param do_vacuum: flag for forcing the vacuuming of the internal database when cleaning the repository. | ||
:param compress: flag for compressing the data when packing loose files. Set to ``Compress.AUTO`` if ``True``. |
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.
Ok, I guess we only allow fully uncompressed or auto compression (no options for YES or KEEP), right?
I guess it's OK for the AiiDA interface in order to keep it simple and we can always point people to the Disk-ObjectStore API for more (or expand this API in the future)
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.
Yes, we currently just have a boolean flag because that was all that was supported by the disk objectstore anyway. Don't think we should start making it even more complex now.
* Update `DiskObjectStoreRepositoryBackend.get_info` to use the dataclasses returned by `count_objects` and `get_total_size` directly. * Change `DiskObjectStoreRepositoryBackend.maintain` to always call `clean_storage`, even during live operation of the container. * Change `DiskObjectStoreRepositoryBackend.maintain` to now pass `CompressMode.AUTO` when `compress` is set to `True`.
f34f7f1
to
8306672
Compare
Fixes #6081
DiskObjectStoreRepositoryBackend.get_info
to use thedataclasses returned by
count_objects
andget_total_size
directly.
DiskObjectStoreRepositoryBackend.maintain
to always callclean_storage
, even during live operation of the container.DiskObjectStoreRepositoryBackend.maintain
to now passCompressMode.AUTO
whencompress
is set toTrue
.Blocked by aiidateam/disk-objectstore#155Still need to address a few items of #6081 .