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

create a separate crate for configuration #121

Merged
merged 6 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions .github/workflows/ship.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ jobs:
include:
- runner: ubuntu-20.04
target: x86_64-unknown-linux-gnu
- runner: ubuntu-20.04
target: aarch64-unknown-linux-gnu
linux-packages: gcc-aarch64-linux-gnu
linker: /usr/bin/aarch64-linux-gnu-gcc
- runner: macos-latest
target: x86_64-apple-darwin
- runner: macos-latest
Expand Down
23 changes: 18 additions & 5 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ edition.workspace = true
workspace = true

[dependencies]
ndc-sqlserver = { path = "../ndc-sqlserver" }
ndc-sqlserver-configuration = { path = "../configuration" }

anyhow = "1.0.82"
clap = { version = "4.5.4", features = ["derive", "env"] }
Expand Down
26 changes: 13 additions & 13 deletions crates/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use std::path::PathBuf;

use clap::Subcommand;

use ndc_sqlserver_configuration as configuration;

const UPDATE_ATTEMPTS: u8 = 3;

/// The various contextual bits and bobs we need to run.
Expand Down Expand Up @@ -56,7 +58,7 @@ pub async fn run(command: Command, context: Context) -> anyhow::Result<()> {
async fn initialize(with_metadata: bool, context: Context) -> anyhow::Result<()> {
let configuration_file = context
.context_path
.join(ndc_sqlserver::connector::CONFIGURATION_FILENAME);
.join(configuration::CONFIGURATION_FILENAME);
fs::create_dir_all(&context.context_path)?; // TODO(PY): .await

// refuse to initialize the directory unless it is empty
Expand All @@ -66,10 +68,8 @@ async fn initialize(with_metadata: bool, context: Context) -> anyhow::Result<()>
}

let raw_configuration = match context.uri {
Some(uri) => {
ndc_sqlserver::configuration::RawConfiguration::with_mssql_connection_string(uri)
}
None => ndc_sqlserver::configuration::RawConfiguration::empty(),
Some(uri) => configuration::RawConfiguration::with_mssql_connection_string(uri),
None => configuration::RawConfiguration::empty(),
};
// create the configuration file
fs::write(
Expand All @@ -80,9 +80,9 @@ async fn initialize(with_metadata: bool, context: Context) -> anyhow::Result<()>
// create the jsonschema file
let configuration_jsonschema_file_path = context
.context_path
.join(ndc_sqlserver::connector::CONFIGURATION_JSONSCHEMA_FILENAME);
.join(configuration::CONFIGURATION_JSONSCHEMA_FILENAME);

let output = schemars::schema_for!(ndc_sqlserver::configuration::RawConfiguration);
let output = schemars::schema_for!(configuration::RawConfiguration);
fs::write(
configuration_jsonschema_file_path,
serde_json::to_string_pretty(&output)? + "\n",
Expand Down Expand Up @@ -140,16 +140,16 @@ async fn update(context: Context) -> anyhow::Result<()> {
for _attempt in 1..=UPDATE_ATTEMPTS {
let configuration_file_path = context
.context_path
.join(ndc_sqlserver::connector::CONFIGURATION_FILENAME);
let input: ndc_sqlserver::configuration::RawConfiguration = {
.join(configuration::CONFIGURATION_FILENAME);
let input: configuration::RawConfiguration = {
let configuration_file_contents =
read_config_file_contents(&configuration_file_path).await?;
serde_json::from_str(&configuration_file_contents)?
};
let output = ndc_sqlserver::configuration::configure(&input).await?;
let output = configuration::configure(&input).await?;

// Check that the input file did not change since we started introspecting,
let input_again_before_write: ndc_sqlserver::configuration::RawConfiguration = {
let input_again_before_write: configuration::RawConfiguration = {
let configuration_file_contents =
read_config_file_contents(&configuration_file_path).await?;
serde_json::from_str(&configuration_file_contents)?
Expand Down Expand Up @@ -199,8 +199,8 @@ async fn read_config_file_contents(configuration_file_path: &PathBuf) -> anyhow:
async fn update_uri_from_context(context: &Context) -> anyhow::Result<()> {
let configuration_file_path = context
.context_path
.join(ndc_sqlserver::connector::CONFIGURATION_FILENAME);
let mut input: ndc_sqlserver::configuration::RawConfiguration = {
.join(configuration::CONFIGURATION_FILENAME);
let mut input: configuration::RawConfiguration = {
let configuration_file_contents =
read_config_file_contents(&configuration_file_path).await?;
serde_json::from_str(&configuration_file_contents)?
Expand Down
21 changes: 21 additions & 0 deletions crates/configuration/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[package]
name = "ndc-sqlserver-configuration"
version.workspace = true
edition.workspace = true

[lints]
workspace = true

[dependencies]
query-engine-metadata = { path = "../query-engine/metadata" }
query-engine-execution = { path = "../query-engine/execution" }


schemars = { version = "0.8.16", features = ["smol_str", "preserve_order"] }
serde = "1.0.198"
serde_json = { version = "1.0.116", features = ["raw_value"] }
tiberius = { version = "0.12.2", default-features = false, features = ["rustls"] }
bb8 = "0.8.1"
bb8-tiberius = "0.15.0"
thiserror = "1.0.59"
prometheus = "0.13.3"
2 changes: 2 additions & 0 deletions crates/configuration/src/configuration.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub const CONFIGURATION_FILENAME: &str = "configuration.json";
pub const CONFIGURATION_JSONSCHEMA_FILENAME: &str = "schema.json";
28 changes: 28 additions & 0 deletions crates/configuration/src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//! Errors that can be thrown when processing configuration.

/// The errors that can be thrown when processing configuration.
///
/// This is effectively a copy of the `ParseError` enum in the `ndc-sdk` crate. However, we don't
/// want a dependency on that crate here, as this crate is used by the CLI. Duplicating here and
/// converting the values later means we can avoid that dependency.
Comment on lines +1 to +7
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the ndc-sdk crate introducing the indirect dependency on openssl?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, ndc-sdk depends on reqwest which in turn depends on openssl

#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("parse error on {file_path}:{line}:{column}: {message}")]
ParseError {
file_path: std::path::PathBuf,
line: usize,
column: usize,
message: String,
},
#[error("invalid configuration version, expected 1, got {version} in {file_path}")]
InvalidConfigVersion {
version: u32,
file_path: std::path::PathBuf,
},

#[error("I/O error: {0}")]
IoErrorButStringified(String),

#[error("I/O error: {0}")]
IoError(#[from] std::io::Error),
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
mod configuration;
mod error;
pub mod introspection;
pub mod version1;

pub use error::Error;

pub use configuration::{CONFIGURATION_FILENAME, CONFIGURATION_JSONSCHEMA_FILENAME};

pub use version1::{
configure, create_state, occurring_scalar_types, validate_raw_configuration, Configuration,
InitializationError, RawConfiguration, State,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Configuration and state for our connector.

use crate::configuration::introspection;
use ndc_sdk::connector;
use super::introspection;
use crate::error::Error;
use query_engine_execution::metrics;
use query_engine_metadata::metadata;
use query_engine_metadata::metadata::{database, Nullable};
Expand Down Expand Up @@ -79,19 +79,14 @@ pub struct State {

/// Validate the user configuration.
pub async fn validate_raw_configuration(
file_path: PathBuf,
config: RawConfiguration,
) -> Result<Configuration, connector::ParseError> {
) -> Result<Configuration, Error> {
if config.version != 1 {
return Err(connector::ParseError::ValidateError(
connector::InvalidNodes(vec![connector::InvalidNode {
file_path: PathBuf::from("file_path"), // TODO(PY): find the path for the config
node_path: vec![connector::KeyOrIndex::Key("version".into())], //tODO(PY)
message: format!(
"invalid configuration version, expected 1, got {0}",
config.version
),
}]),
));
return Err(Error::InvalidConfigVersion {
version: config.version,
file_path: file_path.clone(),
});
}
Ok(Configuration { config })
}
Expand Down Expand Up @@ -140,9 +135,7 @@ async fn select_first_row(
}

/// Construct the deployment configuration by introspecting the database.
pub async fn configure(
configuration: &RawConfiguration,
) -> Result<RawConfiguration, connector::ParseError> {
pub async fn configure(configuration: &RawConfiguration) -> Result<RawConfiguration, Error> {
let mssql_pool = create_mssql_pool(configuration).await.unwrap();

let mut metadata = query_engine_metadata::metadata::Metadata::default();
Expand Down
7 changes: 3 additions & 4 deletions crates/ndc-sqlserver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,16 @@ query-engine-metadata = {path = "../query-engine/metadata"}
query-engine-sql = { path = "../query-engine/sql" }
query-engine-translation = { path = "../query-engine/translation" }

ndc-sqlserver-configuration = { path = "../configuration" }


tiberius = { version = "0.12.2", default-features = false, features = ["rustls"] }
bb8 = "0.8.1"
bb8-tiberius = "0.15.0"
async-trait = "0.1.80"
schemars = { version = "0.8.16", features = ["smol_str"] }
serde = { version = "1.0.198", features = ["derive", "rc"] }
serde_json = { version = "1.0.116", features = ["raw_value"] }
tokio = { version = "1.37.0", features = ["full"] }
tracing = "0.1.40"
prometheus = "0.13.3"
thiserror = "1.0"

[dev-dependencies]
ndc-models = { workspace = true }
Expand Down
39 changes: 33 additions & 6 deletions crates/ndc-sqlserver/src/connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,14 @@ use ndc_sdk::json_response::JsonResponse;
use ndc_sdk::models;
use tokio::fs;

use super::configuration;
use super::explain;
use super::query;
use super::schema;
use ndc_sqlserver_configuration as configuration;

#[derive(Clone, Default)]
pub struct SQLServer {}

pub const CONFIGURATION_FILENAME: &str = "configuration.json";
pub const CONFIGURATION_JSONSCHEMA_FILENAME: &str = "schema.json";

#[async_trait]
impl connector::ConnectorSetup for SQLServer {
type Connector = SQLServer;
Expand All @@ -36,7 +33,9 @@ impl connector::ConnectorSetup for SQLServer {
configuration_dir: impl AsRef<Path> + Send,
) -> Result<<Self::Connector as connector::Connector>::Configuration, connector::ParseError>
{
let configuration_file = configuration_dir.as_ref().join(CONFIGURATION_FILENAME);
let configuration_file = configuration_dir
.as_ref()
.join(configuration::CONFIGURATION_FILENAME);
let configuration_file_contents =
fs::read_to_string(&configuration_file)
.await
Expand All @@ -55,9 +54,37 @@ impl connector::ConnectorSetup for SQLServer {
})
})?;

configuration::validate_raw_configuration(configuration)
configuration::validate_raw_configuration(configuration_file, configuration)
.await
.map(Arc::new)
.map_err(|error| match error {
configuration::Error::ParseError {
file_path,
line,
column,
message,
} => connector::ParseError::ParseError(connector::LocatedError {
file_path,
line,
column,
message,
}),
configuration::Error::InvalidConfigVersion { version, file_path } => {
connector::ParseError::ValidateError(connector::InvalidNodes(vec![
connector::InvalidNode {
file_path,
node_path: vec![connector::KeyOrIndex::Key("version".into())],
message: format!(
"invalid configuration version, expected 1, got {version}",
),
},
]))
}
configuration::Error::IoError(inner) => connector::ParseError::IoError(inner),
configuration::Error::IoErrorButStringified(inner) => {
connector::ParseError::Other(inner.into())
}
})
}

/// Initialize the connector's in-memory state.
Expand Down
3 changes: 1 addition & 2 deletions crates/ndc-sqlserver/src/explain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ use tracing::{info_span, Instrument};

use ndc_sdk::connector;
use ndc_sdk::models;
use ndc_sqlserver_configuration as configuration;
use query_engine_execution::execution;
use query_engine_sql::sql;
use query_engine_translation::translation;

use super::configuration;

/// Explain a query by creating an execution plan
///
/// This function implements the [explain endpoint](https://hasura.github.io/ndc-spec/specification/explain.html)
Expand Down
1 change: 0 additions & 1 deletion crates/ndc-sqlserver/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
pub mod configuration;
pub mod connector;
pub mod explain;
pub mod query;
Expand Down
2 changes: 1 addition & 1 deletion crates/ndc-sqlserver/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
//! See the Hasura
//! [Native Data Connector Specification](https://hasura.github.io/ndc-spec/specification/queries/index.html)
//! for further details.
use super::configuration;
use ndc_sdk::connector;
use ndc_sdk::json_response::JsonResponse;
use ndc_sdk::models;
use ndc_sqlserver_configuration as configuration;
use query_engine_execution::execution;
use query_engine_sql::sql;
use query_engine_translation::translation;
Expand Down
2 changes: 1 addition & 1 deletion crates/ndc-sqlserver/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use ndc_sdk::connector;
use ndc_sdk::models;
use query_engine_metadata::metadata;

use super::configuration;
use ndc_sqlserver_configuration as configuration;

/// Get the connector's schema.
///
Expand Down
Loading
Loading