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 configurability to checks commands #225

Merged
merged 10 commits into from
Feb 8, 2021
2 changes: 2 additions & 0 deletions crates/rover-client/src/query/graph/check.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ mutation CheckSchemaQuery(
$variant: String
$schema: String
$gitContext: GitContextInput!
$config: HistoricQueryParameters!
) {
service(id: $graphId) {
checkSchema(
proposedSchemaDocument: $schema
baseSchemaTag: $variant
gitContext: $gitContext
historicParameters: $config
) {
targetUrl
diffToPrevious {
Expand Down
1 change: 1 addition & 0 deletions crates/rover-client/src/query/graph/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use graphql_client::*;

use reqwest::Url;

type Timestamp = String;
#[derive(GraphQLQuery)]
// The paths are relative to the directory where your `Cargo.toml` is located.
// Both json and the GraphQL schema language are supported as sources for the schema
Expand Down
2 changes: 2 additions & 0 deletions crates/rover-client/src/query/subgraph/check.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@
$implementingServiceName: String!
$partialSchema: PartialSchemaInput!
$gitContext: GitContextInput!
$config: HistoricQueryParameters!
) {
service(id: $graph_id) {
checkPartialSchema(
graphVariant: $variant
implementingServiceName: $implementingServiceName
partialSchema: $partialSchema
gitContext: $gitContext
historicParameters: $config
) {
compositionValidationResult {
errors {
Expand Down
1 change: 1 addition & 0 deletions crates/rover-client/src/query/subgraph/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use graphql_client::*;

use reqwest::Url;

type Timestamp = String;
#[derive(GraphQLQuery)]
// The paths are relative to the directory where your `Cargo.toml` is located.
// Both json and the GraphQL schema language are supported as sources for the schema
Expand Down
30 changes: 29 additions & 1 deletion src/command/graph/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ use crate::client::StudioClientConfig;
use crate::command::RoverStdout;
use crate::git::GitContext;
use crate::utils::loaders::load_schema_from_flag;
use crate::utils::parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource};
use crate::utils::parsers::{
parse_graph_ref, parse_query_count_threshold, parse_query_percentage_threshold,
parse_schema_source, parse_validation_period, GraphRef, SchemaSource, ValidationPeriod,
};
use crate::Result;

#[derive(Debug, Serialize, StructOpt)]
Expand All @@ -29,6 +32,21 @@ pub struct Check {
#[structopt(long, short = "s", parse(try_from_str = parse_schema_source))]
#[serde(skip_serializing)]
schema: SchemaSource,

/// Minimum number of requests within the time window for a query to be
/// considered
JakeDawkins marked this conversation as resolved.
Show resolved Hide resolved
#[structopt(long, parse(try_from_str = parse_query_count_threshold))]
query_count_threshold: Option<i64>,
JakeDawkins marked this conversation as resolved.
Show resolved Hide resolved

/// Minimum percentage of requests within the time window for a query to be
/// considered, relative to total request count. Expected values are 0-5
JakeDawkins marked this conversation as resolved.
Show resolved Hide resolved
/// (i.e. minimum 5% of total request volume)
#[structopt(long, parse(try_from_str = parse_query_percentage_threshold))]
query_percentage_threshold: Option<f64>,
JakeDawkins marked this conversation as resolved.
Show resolved Hide resolved

/// Size of the time window with which to validate schema against (in seconds)
#[structopt(long, parse(try_from_str = parse_validation_period))]
validation_period: Option<ValidationPeriod>,
JakeDawkins marked this conversation as resolved.
Show resolved Hide resolved
}

impl Check {
Expand All @@ -45,6 +63,16 @@ impl Check {
variant: Some(self.graph.variant.clone()),
schema: Some(sdl),
git_context: git_context.into(),
config: check::check_schema_query::HistoricQueryParameters {
query_count_threshold: self.query_count_threshold,
query_count_threshold_percentage: self.query_percentage_threshold,
from: self.validation_period.clone().unwrap_or_default().from,
to: self.validation_period.clone().unwrap_or_default().to,
// we don't support configuring these, but we can't leave them out
excluded_clients: None,
ignored_operations: None,
included_variants: None,
},
},
&client,
)?;
Expand Down
30 changes: 29 additions & 1 deletion src/command/subgraph/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ use crate::client::StudioClientConfig;
use crate::command::RoverStdout;
use crate::git::GitContext;
use crate::utils::loaders::load_schema_from_flag;
use crate::utils::parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource};
use crate::utils::parsers::{
parse_graph_ref, parse_query_count_threshold, parse_query_percentage_threshold,
parse_schema_source, parse_validation_period, GraphRef, SchemaSource, ValidationPeriod,
};

#[derive(Debug, Serialize, StructOpt)]
pub struct Check {
Expand All @@ -34,6 +37,21 @@ pub struct Check {
#[structopt(long, short = "s", parse(try_from_str = parse_schema_source))]
#[serde(skip_serializing)]
schema: SchemaSource,

/// Minimum number of requests within the time window for a query to be
/// considered
#[structopt(long, parse(try_from_str = parse_query_count_threshold))]
query_count_threshold: Option<i64>,

/// Minimum percentage of requests within the time window for a query to be
/// considered, relative to total request count. Expected values are 0-5
/// (i.e. minimum 5% of total request volume)
#[structopt(long, parse(try_from_str = parse_query_percentage_threshold))]
query_percentage_threshold: Option<f64>,

/// Size of the time window with which to validate schema against (in seconds)
#[structopt(long, parse(try_from_str = parse_validation_period))]
validation_period: Option<ValidationPeriod>,
}

impl Check {
Expand All @@ -59,6 +77,16 @@ impl Check {
partial_schema,
implementing_service_name: self.subgraph.clone(),
git_context: git_context.into(),
config: check::check_partial_schema_query::HistoricQueryParameters {
query_count_threshold: self.query_count_threshold,
query_count_threshold_percentage: self.query_percentage_threshold,
from: self.validation_period.clone().unwrap_or_default().from,
to: self.validation_period.clone().unwrap_or_default().to,
// we don't support configuring these, but we can't leave them out
excluded_clients: None,
ignored_operations: None,
included_variants: None,
},
},
&client,
)?;
Expand Down
42 changes: 42 additions & 0 deletions src/utils/parsers.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use regex::Regex;
use serde::Serialize;
use std::{fmt, path::PathBuf};

use crate::{anyhow, Result};
Expand Down Expand Up @@ -60,6 +61,47 @@ pub fn parse_graph_ref(graph_id: &str) -> Result<GraphRef> {
}
}

#[derive(Debug, Serialize, Default, Clone)]
pub struct ValidationPeriod {
pub from: Option<String>,
JakeDawkins marked this conversation as resolved.
Show resolved Hide resolved
pub to: Option<String>,
}

/// Validation period is a positive number of seconds to validate in the past.
JakeDawkins marked this conversation as resolved.
Show resolved Hide resolved
// We just need to validate and negate it
pub fn parse_validation_period(period: &str) -> Result<ValidationPeriod> {
let window = period.parse::<i64>()?;
if window > 0 {
Ok(ValidationPeriod {
from: Some(format!("{}", -window)),
to: Some("-1".to_string()),
JakeDawkins marked this conversation as resolved.
Show resolved Hide resolved
})
} else {
Err(
anyhow!("Invalid validation period. Must be a positive number of seconds.".to_string())
.into(),
)
JakeDawkins marked this conversation as resolved.
Show resolved Hide resolved
}
}

pub fn parse_query_count_threshold(threshold: &str) -> Result<i64> {
let threshold = threshold.parse::<i64>()?;
if threshold < 1 {
Err(anyhow!("Invalid value for query count threshold. Must be a positive integer.").into())
} else {
Ok(threshold)
}
}

pub fn parse_query_percentage_threshold(threshold: &str) -> Result<f64> {
let threshold = threshold.parse::<i64>()?;
if threshold <= 0 || threshold >= 100 {
JakeDawkins marked this conversation as resolved.
Show resolved Hide resolved
Err(anyhow!("Invalid value for query percentage threshold. Must be a positive integer greater than 0 and less than 100").into())
} else {
Ok((threshold / 100) as f64)
}
}

#[cfg(test)]
mod tests {
use super::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource};
Expand Down