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 metric query endpoint #8

Merged
merged 8 commits into from
Nov 14, 2019
Merged

Add metric query endpoint #8

merged 8 commits into from
Nov 14, 2019

Conversation

kamikaz1k
Copy link
Collaborator

@kamikaz1k kamikaz1k commented Nov 11, 2019

Work items

  • offset pagination (W E B S C A L E)
  • datetime range query (start_datetime - end_datetime)
  • json-path selection of arbitrary data is NOT part of this PR

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 and BoxableExpression 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 a QueryableByName to still figure out the queried result can be directly instantiated into diesel Model types. 👌

Copy link
Owner

@woodgern woodgern left a 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);
Copy link
Owner

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?

Copy link
Collaborator Author

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.

@kamikaz1k kamikaz1k changed the title [WIP] Add metric query endpoint Add metric query endpoint Nov 13, 2019
if created_at_start_parsed.is_ok() {
filter_clause.insert_str(
filter_clause.len(),
&format!(" AND created_at > '{}'", created_at_start).to_string()
Copy link
Owner

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

Copy link
Owner

Choose a reason for hiding this comment

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

Not critical btw

if created_at_end_parsed.is_ok() {
filter_clause.insert_str(
filter_clause.len(),
&format!(" AND created_at < '{}'", created_at_end).to_string()
Copy link
Owner

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)

Copy link
Owner

@woodgern woodgern left a 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

@kamikaz1k kamikaz1k merged commit 1255c02 into master Nov 14, 2019
@kamikaz1k kamikaz1k deleted the add-metric-query-endpoint branch November 14, 2019 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants