Skip to content

Commit

Permalink
Fix dev mode when you don't specify a configuration file (#1808)
Browse files Browse the repository at this point in the history
close #1801 

close #1802

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
  • Loading branch information
bnjjj authored Sep 16, 2022
1 parent be64832 commit e97193a
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 11 deletions.
12 changes: 9 additions & 3 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,20 +72,26 @@ By [@o0Ignition0o](https://github.com/o0Ignition0o) in https://github.com/apollo

## 🚀 Features

### Automatically check "Return Query Plans from Router" checkbox in Sandbox ([Issue #](https://github.com/apollographql/router/issues/1803))
### Automatically check "Return Query Plans from Router" checkbox in Sandbox ([Issue #1803](https://github.com/apollographql/router/issues/1803))

When loading Sandbox, we now automatically configure it to toggle the "Request query plans from Router" checkbox to the enabled position which requests query plans from the Apollo Router when executing operations. These query plans are displayed in the Sandbox interface and can be seen by selecting "Query Plan Preview" from the drop-down above the panel on the right side of the interface.

By [@abernix](https://github.com/abernix) in https://github.com/apollographql/router/pull/1804

## 🐛 Fixes

### Respect supergraph path for kubernetes deployment probes (#1787)
### Fix dev mode when you don't specify a configuration file ([Issue #1801](https://github.com/apollographql/router/issues/1801)) ([Issue #1802](https://github.com/apollographql/router/issues/1802))

Previously the dev mode was ignored if you ran the router without a configuration file.

By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/1808

### Respect supergraph path for kubernetes deployment probes ([Issue #1787](https://github.com/apollographql/router/issues/1787))

For cases where you configured the `supergraph.path` for the router when using the helm chart, the liveness
and readiness probes continued to use the default path of `/` and so the start failed.

By @damienpontifex in #1788
By [@damienpontifex](https://github.com/damienpontifex) in https://github.com/apollographql/router/pull/1788

### Get variable default values from the query for query plan condition nodes ([PR #1640](https://github.com/apollographql/router/issues/1640))

Expand Down
4 changes: 2 additions & 2 deletions apollo-router/src/configuration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ pub struct Configuration {

// Dev mode
#[serde(skip)]
dev: Option<bool>,
pub(crate) dev: Option<bool>,
}

impl<'de> serde::Deserialize<'de> for Configuration {
Expand Down Expand Up @@ -365,7 +365,7 @@ impl Configuration {
}

impl Configuration {
fn validate(self) -> Result<Self, ConfigurationError> {
pub(crate) fn validate(self) -> Result<Self, ConfigurationError> {
// Sandbox and Homepage cannot be both enabled
if self.sandbox.enabled && self.homepage.enabled {
return Err(ConfigurationError::InvalidConfiguration {
Expand Down
45 changes: 41 additions & 4 deletions apollo-router/src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,20 @@ impl ConfigurationSource {
/// Convert this config into a stream regardless of if is static or not. Allows for unified handling later.
fn into_stream(self) -> impl Stream<Item = Event> {
match self {
ConfigurationSource::Static(instance) => {
ConfigurationSource::Static(mut instance) => {
if instance.dev.unwrap_or_default() {
instance.enable_dev_mode();
}
stream::iter(vec![UpdateConfiguration(instance)]).boxed()
}
ConfigurationSource::Stream(stream) => {
stream.map(|x| UpdateConfiguration(Box::new(x))).boxed()
}
ConfigurationSource::Stream(stream) => stream
.map(|mut x| {
if x.dev.unwrap_or_default() {
x.enable_dev_mode();
}
UpdateConfiguration(Box::new(x))
})
.boxed(),
ConfigurationSource::File {
path,
watch,
Expand Down Expand Up @@ -731,6 +739,7 @@ mod tests {
_ => panic!("the event from the stream must be UpdateConfiguration"),
};
assert!(cfg.supergraph.introspection);
assert!(!cfg.homepage.enabled);
assert!(cfg.sandbox.enabled);
assert!(cfg.plugins().iter().any(
|(name, val)| name == "experimental.expose_query_plan" && val == &Value::Bool(true)
Expand All @@ -740,6 +749,7 @@ mod tests {
.iter()
.any(|(name, val)| name == "apollo.include_subgraph_errors"
&& val == &json!({"all": true})));
cfg.validate().unwrap();

// Modify the file and try again
write_and_flush(&mut file, contents).await;
Expand All @@ -748,6 +758,7 @@ mod tests {
_ => panic!("the event from the stream must be UpdateConfiguration"),
};
assert!(cfg.supergraph.introspection);
assert!(!cfg.homepage.enabled);
assert!(cfg.sandbox.enabled);
assert!(cfg.plugins().iter().any(
|(name, val)| name == "experimental.expose_query_plan" && val == &Value::Bool(true)
Expand All @@ -757,12 +768,38 @@ mod tests {
.iter()
.any(|(name, val)| name == "apollo.include_subgraph_errors"
&& val == &json!({"all": true})));
cfg.validate().unwrap();

// This time write garbage, there should not be an update.
write_and_flush(&mut file, ":garbage").await;
assert!(stream.into_future().now_or_never().is_none());
}

#[tokio::test(flavor = "multi_thread")]
async fn config_dev_mode_without_file() {
let mut stream =
ConfigurationSource::from(Configuration::builder().dev(true).build().unwrap())
.into_stream()
.boxed();

let cfg = match stream.next().await.unwrap() {
UpdateConfiguration(configuration) => configuration,
_ => panic!("the event from the stream must be UpdateConfiguration"),
};
assert!(cfg.supergraph.introspection);
assert!(cfg.sandbox.enabled);
assert!(!cfg.homepage.enabled);
assert!(cfg.plugins().iter().any(
|(name, val)| name == "experimental.expose_query_plan" && val == &Value::Bool(true)
));
assert!(cfg
.plugins()
.iter()
.any(|(name, val)| name == "apollo.include_subgraph_errors"
&& val == &json!({"all": true})));
cfg.validate().unwrap();
}

#[tokio::test(flavor = "multi_thread")]
async fn config_by_file_invalid() {
let (path, mut file) = create_temp_file();
Expand Down
4 changes: 2 additions & 2 deletions apollo-router/src/router_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,8 @@ async fn create_plugins(
.iter()
.map(|(name, plugin)| (name, plugin.name()))
.collect::<Vec<(&String, &str)>>();
tracing::info!(
"enabled plugins: {:?}",
tracing::debug!(
"plugins list: {:?}",
plugin_details
.iter()
.map(|(name, _)| name)
Expand Down

0 comments on commit e97193a

Please sign in to comment.