Skip to content
This repository was archived by the owner on Aug 2, 2023. It is now read-only.

Add a mini-language parser for generic GQL list filtering #305

Merged
merged 15 commits into from
Aug 28, 2020

Conversation

Jaymel
Copy link
Contributor

@Jaymel Jaymel commented Jul 27, 2020

Defined a method to change the query statement to sql statement.

@Jaymel Jaymel added the feature label Jul 27, 2020
@Jaymel Jaymel requested a review from achimnol July 27, 2020 03:27
@CLAassistant
Copy link

CLAassistant commented Jul 27, 2020

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jul 27, 2020

Codecov Report

Merging #305 into master will increase coverage by 1.15%.
The diff coverage is 88.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #305      +/-   ##
==========================================
+ Coverage   49.42%   50.57%   +1.15%     
==========================================
  Files          42       46       +4     
  Lines        6503     7786    +1283     
==========================================
+ Hits         3214     3938     +724     
- Misses       3289     3848     +559     
Impacted Files Coverage Δ
.../ai/backend/manager/models/minilang/queryfilter.py 88.00% <88.00%> (ø)
src/ai/backend/manager/models/domain.py 62.25% <0.00%> (-5.02%) ⬇️
src/ai/backend/gateway/auth.py 50.00% <0.00%> (-4.42%) ⬇️
src/ai/backend/manager/models/user.py 34.65% <0.00%> (-0.17%) ⬇️
src/ai/backend/manager/plugin/exceptions.py 100.00% <0.00%> (ø)
src/ai/backend/manager/exceptions.py 100.00% <0.00%> (ø)
src/ai/backend/manager/plugin/error_monitor.py 50.00% <0.00%> (ø)
src/ai/backend/gateway/exceptions.py 88.03% <0.00%> (+0.47%) ⬆️
src/ai/backend/manager/models/group.py 49.22% <0.00%> (+2.13%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c946ef3...cb6ad1e. Read the comment docs.

@achimnol
Copy link
Member

While I'm reviewing, please sign up the CLA and fix up the style issues indicated by lint checkers.

@achimnol achimnol added this to the 20.09 milestone Jul 28, 2020
Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

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

Please check out the comments.
Also, where are the code changes for GQL list queries in other modules of ai.backend.manager.models?

Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

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

Overall, LGTM about fulfilling my functional requirements.
Since your internship has finished, I will do some style/code clean up.

@achimnol achimnol changed the title Feature/add graphql translater Add a mini-language parser for generic GQL list filtering Aug 28, 2020
@achimnol
Copy link
Member

achimnol commented Aug 28, 2020

Now we need to use ai.backend.manager.models.minilang.queryfilter.QueryFilterParser in all paginated list GQL query resolver functions under ai.backend.manager.models, adding an optional string-type "filter" argument to all such queries.

@achimnol
Copy link
Member

I will file a new issue for the application of QueryFilterParser, after merging this.

@achimnol achimnol merged commit 3b737e2 into master Aug 28, 2020
@achimnol achimnol deleted the feature/add-graphql-translater branch August 28, 2020 13:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants