-
Notifications
You must be signed in to change notification settings - Fork 7
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: add filtering in assets api #100
Conversation
546e1f6
to
1c916a7
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.
Also we need to update AssetRepository.GetCount
to also read and process the same filter config as GetAll
. Or else we will get wrong total value.
If it is for filtering, why not using The nested query params is in the wrong format. |
Yes, we can go with the |
@mabdh @StewartJingga, the proto file needs to be updated to pass correct config in |
Why there is |
We can just fix the proto in this PR instead, we dont want breaking test on main branch. @scortier |
Recently, updated the query param to this format : |
For the proto, in that case I think this PR could also covers this issue #105 @scortier @StewartJingga or as an alternative, please create different AssetConfig struct for grpc and http. Later in next issue we can unify it. |
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.
LGTM
Added filtering by certain fields in assets api.
Querystring: "?types=table&services=bigquery&size=30&offset=50&sort=created_at&direction=desc&data[dataset]=booking&data[project]=p-godata-id&q=internal&q_fields=name,urn,description,services",
types=topic,table
services=kafka,postgres
data[dataset]=booking&data[project]=p-godata-id"
q=internal&q_fields=name,urn,description,services
sort=created_at
direction=desc