Skip to content
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

Add basic metadata options to CLI #293

Merged
merged 12 commits into from
Dec 2, 2024
Merged

Add basic metadata options to CLI #293

merged 12 commits into from
Dec 2, 2024

Conversation

qubixes
Copy link
Collaborator

@qubixes qubixes commented Nov 29, 2024

Adds:

  • ibridges meta-show: show the metadata for a collection or dataobject.
  • ibridges meta-add: Add one new metadata entry for a collection or dataobject.
  • ibridges meta-del: Delete one, multiple or all metadata items using the key/value/units.
  • ibridges list --metadata: List the collection, including all metadata of the listed items.
  • ibridges list --short: Mimics Unix ls with no options.
  • ibridges list --long: Shows checksums and sizes of data objects.
  • ibridges ls alias for ibridges list
  • ibridges meta alias for ibridges meta-list

Deprecates:

  • MetaData.set: I think we should remove this method for 2.0, since the behavior can be easily emulated with two commands (delete/add).

Improvements:

  • IrodsPath.walk: Performance with depth==1 should be improved for large collections.

TODO:

  • Documentation readthedocs
  • CLI help
  • New tests

@qubixes qubixes requested a review from chStaiger November 29, 2024 10:19
_list_coll(session, _parse_remote(args.remote_path, session), args.metadata)


def ibridges_meta_show():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be consistent with the other iRODS clients we should rename show into list.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Collaborator

@chStaiger chStaiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice PR, I like it. I have nothing to complain about in the code

@qubixes
Copy link
Collaborator Author

qubixes commented Nov 29, 2024

Nice PR, I like it. I have nothing to complain about in the code

I have added some more features and a performance improvement for walk with depth==1.

@@ -653,6 +657,8 @@ def _get_data_objects(
"""
# all objects in the collection
objs = [(obj.collection.path, obj.name, obj.size, obj.checksum) for obj in coll.data_objects]
if depth == 1:
return objs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!!! Really a good catch!

@qubixes
Copy link
Collaborator Author

qubixes commented Nov 29, 2024

@chStaiger Do you understand why the tests fail? I think it's because there are two sessions running at the same time, and there is some timing issue, where the change from the cli doesn't trickle through to the other running session. How do you want to fix this?

@chStaiger
Copy link
Collaborator

I think this exception is thrown when you try to delete some metadata that is not there. So there must be some confusion with the metadata as we use it in the metadata tests and the metadata in cli.

from ibridges.path import IrodsPath
from ibridges.interactive import interactive_auth

session = interactive_auth()
irods_path = IrodsPath(session, '~', 'comicbooks')
coll_meta = irods_path.collection.metadata

coll_meta.remove("bogus", "bogus")

CAT_SUCCESS_BUT_WITH_NO_INFO: None

@chStaiger
Copy link
Collaborator

I exchanged the assertion in test_cli_meta

irods-client_1 | @mark.parametrize("item_name", ["collection", "dataobject"])
irods-client_1 | def test_meta_cli(item_name, request, pass_opts):
irods-client_1 | item = request.getfixturevalue(item_name)
irods-client_1 | meta = MetaData(item)
irods-client_1 | meta.clear()
irods-client_1 | cli_path = f"irods:{item.path}"
irods-client_1 |
irods-client_1 | ret = subprocess.run(["ibridges", "meta-list", cli_path], capture_output=True, **pass_opts)
irods-client_1 | assert len(ret.stdout.strip("\n").split("\n")) == 1
irods-client_1 |
irods-client_1 | subprocess.run(["ibridges", "meta-add", cli_path, "key", "value", "units"], **pass_opts)
irods-client_1 | assert ("key", "value", "units") in meta
irods-client_1 |
irods-client_1 | subprocess.run(["ibridges", "meta-del", cli_path, "--key", "key"], **pass_opts)
irods-client_1 | > assert ("key", "value", "units") not in meta
irods-client_1 | E AssertionError: assert ('key', 'value', 'units') not in MetaData</tempZone/home/rods/test_collection>
irods-client_1 |
irods-client_1 | test_cli.py:195: AssertionError


So the error seems to be that the metadata item is not removed.

@chStaiger
Copy link
Collaborator

I think I found it.

The meta is a snapshot at a time and not updated when the metadata gets changed on the server.
So before all of the asserts we need to fetch the current meta

@chStaiger
Copy link
Collaborator

To refresh the meta variable we need the session to fetch the information from the server.

item = IrodsPath(session, <path>).<Obj|Coll>
meta = Metadata(item)
assert ...

@qubixes
Copy link
Collaborator Author

qubixes commented Dec 2, 2024

@chStaiger I have added a refresh method to the MetaData class. I think for now this will work fine. We can think of allowing an irodspath at some other time.

@chStaiger
Copy link
Collaborator

@chStaiger I have added a refresh method to the MetaData class. I think for now this will work fine. We can think of allowing an irodspath at some other time.

Thanks!!! I agree. Noted down for iBridges 2.0.0 ;)

Copy link
Collaborator

@chStaiger chStaiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved!!!!

@qubixes qubixes merged commit 0594b44 into develop Dec 2, 2024
12 checks passed
@qubixes qubixes deleted the meta-cli branch December 2, 2024 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants