-
Notifications
You must be signed in to change notification settings - Fork 2
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 metric query endpoint #8
Conversation
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.
Word, I'm currently looking at parsing the QL and turning it into a diesel based query
|
||
println!("### QUERY: SELECT * FROM metrics {};", filter_clause); | ||
|
||
let query_string = format!("SELECT * FROM metrics {} ORDER BY id LIMIT 10", filter_clause); |
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.
Any reason for limit 10?
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.
to test pagination, we can change it, nbd.
metrix/src/main.rs
Outdated
if created_at_start_parsed.is_ok() { | ||
filter_clause.insert_str( | ||
filter_clause.len(), | ||
&format!(" AND created_at > '{}'", created_at_start).to_string() |
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.
Might want >=
here? Totally a 'product' thing, but I feel like 'start_date' is inclusive
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.
Not critical btw
metrix/src/main.rs
Outdated
if created_at_end_parsed.is_ok() { | ||
filter_clause.insert_str( | ||
filter_clause.len(), | ||
&format!(" AND created_at < '{}'", created_at_end).to_string() |
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.
Same here (not at all critical)
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, Let's Get This Merged
Work items
offset
pagination (W E B S C A L E)start_datetime
-end_datetime
)Query format:
/metrics?offset=4&start_datetime=2019-11-11T01:10:00&end_datetime=2019-11-11T01:18:00
It works with any combination of the 3 parameters.
Dev Notes (historical):
Originally tried to do dynamic query segments using the ORM using
Box
andBoxableExpression
types (see SO answer here).It was very complicated, but moreover we found out that json operators haven't yet been implemented for
diesel
yet. See diesel-rs/diesel#44.So we instead are going with
sql_query(...)
to generate a raw SQL query. Luckily diesel provides aQueryableByName
to still figure out the queried result can be directly instantiated into diesel Model types. 👌