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 UserTags support to list_objects API #1381

Merged
merged 18 commits into from
Jan 13, 2024

Conversation

angrybat
Copy link
Contributor

@angrybat angrybat commented Dec 27, 2023

This adds the tags property to the Object class which can be set in the constructor of the class. This defaults to an empty Tags class.

The http response already contains the tags in the xml response under the text in the "UserTags" key. The fromxml method has been modified to set the tags to a populated version of the Tags class. This used the added Tags.object_from_user_tags_text method.

The Tags.object_from_user_tags_text method is a new method that creates a Tags class instance for Objects from a string in the format "key1=value1&key2=value2". This is the format the tags are in the text of the "UserTags" key.

Copy link
Member

@balamurugana balamurugana left a comment

Choose a reason for hiding this comment

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

Filtering objects by any parameter should be done outside of minio-py. Please keep the PR focusing on adding UserTags support.

@angrybat
Copy link
Contributor Author

@balamurugana out of interest how come filtering by UserTags should be done outside of minio-py?

The list_objects function has a prefix parameter that allows you to filter by path. How come that is different to filtering by UserTags?

The filtering I have added here is optional. So the original functionality of the function is preserved.

@balamurugana
Copy link
Member

@balamurugana out of interest how come filtering by UserTags should be done outside of minio-py?

The list_objects function has a prefix parameter that allows you to filter by path. How come that is different to filtering by UserTags?

The filtering I have added here is optional. So the original functionality of the function is preserved.

Filtering by prefix and other parameters are done at server side whereas UserTags filtering is done at client side in this PR which we don't want.

@angrybat angrybat force-pushed the list_objects_can_filter_by_tags branch from 7bcf676 to 9c6d2d6 Compare December 28, 2023 14:01
@angrybat angrybat changed the title list_objects can filter by tags Set UserTags when calling list_bucket Dec 28, 2023
@@ -132,6 +133,7 @@ def __init__( # pylint: disable=too-many-arguments
owner_name: str | None = None,
content_type: str | None = None,
is_delete_marker: bool = False,
tags: Tags = Tags.new_object_tags(),
Copy link
Member

Choose a reason for hiding this comment

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

It should be tags: Tags | None = None

@@ -83,6 +83,16 @@ def fromxml(cls: Type[A], element: ET.Element) -> A:
obj[key] = value
return obj

@classmethod
def object_from_user_tags_text(cls: Type[A], string: str | None) -> A:
Copy link
Member

Choose a reason for hiding this comment

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

@balamurugana balamurugana changed the title Set UserTags when calling list_bucket Add UserTags support to list_objects API Dec 28, 2023
minio/datatypes.py Outdated Show resolved Hide resolved
balamurugana
balamurugana previously approved these changes Dec 30, 2023
minio/datatypes.py Outdated Show resolved Hide resolved
minio/datatypes.py Outdated Show resolved Hide resolved
minio/datatypes.py Outdated Show resolved Hide resolved
minio/datatypes.py Outdated Show resolved Hide resolved
Copy link
Member

@balamurugana balamurugana left a comment

Choose a reason for hiding this comment

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

import List, Tuple, Any at line no. 30

@angrybat
Copy link
Contributor Author

Can I get a second review please

@harshavardhana harshavardhana merged commit 67aab07 into minio:master Jan 13, 2024
18 checks passed
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.

3 participants