-
Notifications
You must be signed in to change notification settings - Fork 314
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: limiting the size of query results to Dashboard #3901
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3901 +/- ##
==========================================
- Coverage 85.81% 85.50% -0.31%
==========================================
Files 965 965
Lines 165015 165056 +41
==========================================
- Hits 141602 141135 -467
- Misses 23413 23921 +508 |
931c243
to
55cfd4c
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.
We don't need to define the parse()
method which is equivalent to from_str().ok()
.
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.
If we go forward in the limit
param way, I don't think we should add the RequestSource option that would introduce tech debt.
@MichaelScofield @zyy17 @tisonkun @evenyag PTAL: refactor PR by |
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. Thanks for your contribution!
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
close #3716
What's changed and what's your intention?
Limit the returned rows to 1000, iff the query comes from Dashboard.
Adding a new
source enum
to theSqlQuery
to identify if it's a request from DashBorad or not, and then handle the flow inside thesrc/servers/src/http/handler.rs:72
. In addition, considering that the dashboard only supportsgreptimedb_v1
format, only the output ofgreptimedb_v1
format outputs is processed this PR.Checklist