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 label name filtering to /labelValues endpoints #1530

Closed
willr3 opened this issue Mar 20, 2024 · 8 comments · Fixed by #1534
Closed

add label name filtering to /labelValues endpoints #1530

willr3 opened this issue Mar 20, 2024 · 8 comments · Fixed by #1534
Assignees
Milestone

Comments

@willr3
Copy link
Collaborator

willr3 commented Mar 20, 2024

The /labelValues endpoint on test and run currently return all the labels which can be a prohibitively large amount of data for use cases that only want a few labels.
It would help if users can specify which labels to include (or exclude) as part of the query API.

Current idea is an include and an exclude query parameter that accepts label names

@willr3 willr3 added this to the 0.13 milestone Mar 20, 2024
@willr3 willr3 self-assigned this Mar 20, 2024
@willr3
Copy link
Collaborator Author

willr3 commented Mar 21, 2024

Do we want to repeat the query parameter for each entry

?include=foo&include=bar`)

or do some string transformations and accept string encoded arrays?

?include=["foo","bar"]

@barreiro
Copy link
Collaborator

?include=foo,bar&exclude=other,stuff

seems to de the standard way for parameters of list type

@lampajr
Copy link
Member

lampajr commented Mar 21, 2024

?include=foo,bar&exclude=other,stuff

seems to de the standard way for parameters of list type

+1 I think that comma separated list is the standard for these use cases

@willr3
Copy link
Collaborator Author

willr3 commented Mar 22, 2024

I thought comma separated was handled by jakarta.ws.rs but I was fooled by my System.out.println. jakarta.ws.rs will create two list entries from include=foo&include=bar but include=foo,bar is being treated as a single entry in the list. We could create a custom parsing for it but if , separated was the standard I would expect jakarta.ws to parse it

@lampajr
Copy link
Member

lampajr commented Mar 22, 2024

I thought comma separated was handled by jakarta.ws.rs but I was fooled by my System.out.println. jakarta.ws.rs will create two list entries from include=foo&include=bar but include=foo,bar is being treated as a single entry in the list. We could create a custom parsing for it but if , separated was the standard I would expect jakarta.ws to parse it

IIRC you should specify the separator somehow, that way it should automatically split the string into a list, I remember something like @Separator(",") but I am not sure.

How are you configuring it in yout tests?

@lampajr
Copy link
Member

lampajr commented Mar 22, 2024

I couldn't find any doc but I found a PR mentioning something like that quarkusio/quarkus#29550

@willr3
Copy link
Collaborator Author

willr3 commented Mar 23, 2024

You're right, there is the org.jboss.resteasy.reactive.Separator annotation that adds the ability to specify include=foo,bar. It isn't part of the jakarta.ws spec but Horreum is a quarkus app not a portable jakartaEE app so I think it is safe to add the dependency.
I will check how it impacts the openapi.yaml

@willr3
Copy link
Collaborator Author

willr3 commented Mar 25, 2024

Adding the Separator did not change the openapi.yaml so I've added it to the PR. Users can use include=foo,bar or the jakarata.ws approach include=foo&include=bar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants