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::command::RoverStdout;
use crate::utils::client::StudioClientConfig;
use crate::utils::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,

/// The minimum number of times a query or mutation must have been executed
/// in order to be considered in the check operation
#[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 times a query or mutation must have been executed
/// in the time window, relative to total request count, for it to be
/// considered in the check. Valid numbers are in the range 0 <= x <= 100
#[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>,
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::command::RoverStdout;
use crate::utils::client::StudioClientConfig;
use crate::utils::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,

/// The minimum number of times a query or mutation must have been executed
/// in order to be considered in the check operation
#[structopt(long, parse(try_from_str = parse_query_count_threshold))]
query_count_threshold: Option<i64>,

/// Minimum percentage of times a query or mutation must have been executed
/// in the time window, relative to total request count, for it to be
/// considered in the check. Valid numbers are in the range 0 <= x <= 100
#[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
46 changes: 46 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,51 @@ pub fn parse_graph_ref(graph_id: &str) -> Result<GraphRef> {
}
}

#[derive(Debug, Serialize, Default, Clone)]
pub struct ValidationPeriod {
// these timstamps could be represented as i64, but the API expects
// Option<String>
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.
// We just need to validate and negate it.
//
// Valid windows of time to search are only in the past (negative seconds).
// We only support validating "to" now (-0)
pub fn parse_validation_period(period: &str) -> Result<ValidationPeriod> {
let window = period.parse::<i64>()?;
if window > 0 {
Ok(ValidationPeriod {
// search "from" a negative time window
from: Some(format!("{}", -window)),
// search "to" now (-0) seconds
to: Some("-0".to_string()),
})
} else {
Err(anyhow!("Invalid validation period. Must be a positive number of seconds.").into())
}
}

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 !(0..=100).contains(&threshold) {
Err(anyhow!("Invalid value for query percentage threshold. Valid numbers are in the range 0 <= x <= 100").into())
} else {
Ok((threshold / 100) as f64)
}
}

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