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.

Sign up for GitHub

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

[BGS-150] [messages] when router config doesn’t exist at the path provided in --router-config, stderr should receive message {path} does not exist, creating a router config from CLI options. #2344

Merged
merged 1 commit into from
Jan 9, 2025
Merged

[BGS-150] [messages] when router config doesn’t exist at the path provided in --router-config, stderr should receive message {path} does not exist, creating a router config from CLI options. #2344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/command/dev/next/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,6 @@ impl Dev {

let router_config_path = self.opts.supergraph_opts.router_config_path.clone();

let _config = RunRouterConfig::default()
.with_address(router_address)
.with_config(&read_file_impl, router_config_path.as_ref())
.await
.map_err(|err| RoverError::new(anyhow!("{}", err)))?;

Comment on lines -65 to -70
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This value _config was thrown away; computing this had the side effect of throwing here when the filepath is bad. I want to wait until a later point to handle the filepath issue.

let profile = &self.opts.plugin_opts.profile;
let graph_ref = &self.opts.supergraph_opts.graph_ref;
if let Some(graph_ref) = graph_ref {
Expand Down
78 changes: 41 additions & 37 deletions src/command/dev/next/router/config/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::net::{IpAddr, Ipv4Addr, SocketAddr};
use buildstructor::buildstructor;
use camino::Utf8PathBuf;
use http::Uri;
use rover_std::errln;
use rover_std::{Fs, RoverStdError};
use thiserror::Error;

Expand Down Expand Up @@ -130,44 +131,47 @@ impl RunRouterConfig<RunRouterConfigReadConfig> {
path: Option<&Utf8PathBuf>,
) -> Result<RunRouterConfig<RunRouterConfigFinal>, ReadRouterConfigError> {
match path {
Some(path) => {
Fs::assert_path_exists(&path).map_err(ReadRouterConfigError::Fs)?;

match read_file_impl.read_file(&path).await {
Ok(contents) => {
let yaml = serde_yaml::from_str(&contents).map_err(|err| {
ReadRouterConfigError::Deserialization {
path: path.clone(),
source: err,
}
})?;

let router_config = RouterConfigParser::new(&yaml);
let address = router_config.address()?;
let address = address
.map(RouterAddress::from)
.unwrap_or(self.state.router_address);
let health_check_enabled = router_config.health_check_enabled();
let health_check_endpoint = router_config.health_check_endpoint()?;
let health_check_path = router_config.health_check_path();
let listen_path = router_config.listen_path();

Ok(RunRouterConfigFinal {
listen_path,
address,
health_check_enabled,
health_check_endpoint,
health_check_path,
raw_config: contents.to_string(),
})
}
Err(RoverStdError::EmptyFile { .. }) => Ok(RunRouterConfigFinal::default()),
Err(err) => Err(ReadRouterConfigError::ReadFile {
path: path.clone(),
source: Box::new(err),
}),
Some(path) => match read_file_impl.read_file(&path).await {
Ok(contents) => {
let yaml = serde_yaml::from_str(&contents).map_err(|err| {
ReadRouterConfigError::Deserialization {
path: path.clone(),
source: err,
}
})?;

let router_config = RouterConfigParser::new(&yaml);
let address = router_config.address()?;
let address = address
.map(RouterAddress::from)
.unwrap_or(self.state.router_address);
let health_check_enabled = router_config.health_check_enabled();
let health_check_endpoint = router_config.health_check_endpoint()?;
let health_check_path = router_config.health_check_path();
let listen_path = router_config.listen_path();

Ok(RunRouterConfigFinal {
listen_path,
address,
health_check_enabled,
health_check_endpoint,
health_check_path,
raw_config: contents.to_string(),
})
}
}
Err(RoverStdError::EmptyFile { .. }) => Ok(RunRouterConfigFinal::default()),
Err(RoverStdError::AdhocError { .. }) => {
errln!(
"{} does not exist, creating a router config from CLI options.",
&path
);
Ok(RunRouterConfigFinal::default())
Comment on lines +163 to +168
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here is the new code; rather than assert at the top Fs::assert_path_exists(&path).map_err(ReadRouterConfigError::Fs)?;, here we'll handle this type of error. This allows us to identify the error, but respond gracefully with the default configuration.

}
Err(err) => Err(ReadRouterConfigError::ReadFile {
path: path.clone(),
source: Box::new(err),
}),
},
None => Ok(RunRouterConfigFinal::default()),
}
.map(|state| RunRouterConfig { state })
Expand Down
Loading