-
Notifications
You must be signed in to change notification settings - Fork 88
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
feat: handle multiple tags on datasets and topics apis #3204
base: master
Are you sure you want to change the base?
Conversation
67cc632
to
b1893c0
Compare
@@ -73,7 +73,7 @@ | |||
log = logging.getLogger(__name__) | |||
|
|||
ns = apiv2.namespace("datasets", "Dataset related operations") | |||
search_parser = DatasetSearch.as_request_parser() | |||
search_parser = DatasetSearch.as_request_parser(store_missing=False) |
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.
We use store_missing=False
because we don't want to send filters set to None
to the search service (Mongo does not care).
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.
Excellent work! Very well executed 🙏
I'm slightly confused as it seems we don't have the multi-tags implemented for the topics on the udata-search-service side, am I missing something?
try: | ||
return search.query(Dataset, **multi_to_dict(request.args)) | ||
return search.query(Dataset, **args) |
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.
Makes more sense indeed.
Note for self: we have the same pattern elsewhere
udata/udata/core/reuse/apiv2.py
Lines 29 to 30 in bfeae1b
search_parser.parse_args() return search.query(ReuseSearch, **multi_to_dict(request.args)) udata/udata/core/organization/apiv2.py
Lines 34 to 35 in bfeae1b
search_parser.parse_args() return search.query(OrganizationSearch, **multi_to_dict(request.args)) udata/udata/core/dataservices/apiv2.py
Lines 28 to 29 in bfeae1b
search_parser.parse_args() return search.query(DataserviceSearch, **multi_to_dict(request.args))
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.
Those could be refactored when multi-tags is introduced elsewhere (except for Organizations that don't have tags AFAIK).
url = url + f"&{name}={value}" | ||
return url | ||
|
||
# FIXME: unused? |
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.
Note to self: check, and remove in another cleanup PR if indeed unused.
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.
Thank you a lot for this PR 🚀
I think we can add a changelog entry and we're all clear!
Multiple tags filter is now supported and datasets and topics list APIs.
The filter is a
AND
, i.e.?tag=tag1&tag=tag2
will return objects that have both thetag1
andtag2
tags.datasets/search
through MongoEngine)