Skip to content

Commit 6c295d1

Browse files
danieljharveyhasura-bot
authored andcommitted
Revert "Optional CompatibilityConfig for v3-engine" (#1006)
Reverts hasura/v3-engine#998 We are going to use `opendds` Flags for this instead. V3_GIT_ORIGIN_REV_ID: 155c7a6b17be95a6370b08fdc425791e3eb8b70a
1 parent 3521c39 commit 6c295d1

File tree

14 files changed

+43
-151
lines changed

14 files changed

+43
-151
lines changed

v3/Cargo.lock

-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

v3/changelog.md

-3
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@
44

55
### Added
66

7-
- Allow passing an optional `CompatibiityConfig` file that allows opting in to
8-
new breaking metadata changes.
9-
107
### Fixed
118

129
### Changed

v3/crates/compatibility/Cargo.toml

-2
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,9 @@ license.workspace = true
88
bench = false
99

1010
[dependencies]
11-
metadata-resolve = { path = "../metadata-resolve" }
1211
open-dds = { path = "../open-dds" }
1312
opendds-derive = { path = "../utils/opendds-derive" }
1413

15-
anyhow = { workspace = true }
1614
chrono = { workspace = true }
1715
schemars = { workspace = true }
1816
serde = { workspace = true }

v3/crates/compatibility/src/config.rs

+35-32
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,27 @@
11
use super::compatibility_date::CompatibilityDate;
2-
use super::types::CompatibilityConfig;
32
use chrono::NaiveDate;
4-
use metadata_resolve::configuration::WarningsToRaise;
3+
4+
#[derive(Clone, Debug, PartialEq, serde::Serialize, opendds_derive::OpenDd)]
5+
#[serde(rename_all = "camelCase")]
6+
#[serde(deny_unknown_fields)]
7+
#[opendd(json_schema(id = "v1/CompatibilityConfig"))]
8+
/// The compatibility configuration of the Hasura metadata.
9+
pub struct CompatibilityConfigV1 {
10+
/// Any backwards incompatible changes made to Hasura DDN after this date won't impact the metadata.
11+
pub date: CompatibilityDate,
12+
// TODO: add flags.
13+
}
14+
15+
#[derive(Clone, Debug, PartialEq, serde::Serialize, opendds_derive::OpenDd)]
16+
#[serde(rename_all = "camelCase")]
17+
#[serde(deny_unknown_fields)]
18+
#[opendd(json_schema(id = "v2/CompatibilityConfig"))]
19+
/// The compatibility configuration of the Hasura metadata.
20+
pub struct CompatibilityConfigV2 {
21+
/// Any backwards incompatible changes made to Hasura DDN after this date won't impact the metadata.
22+
pub date: CompatibilityDate,
23+
// TODO: add flags.
24+
}
525

626
pub const fn new_compatibility_date(year: i32, month: u32, day: u32) -> CompatibilityDate {
727
CompatibilityDate(match NaiveDate::from_ymd_opt(year, month, day) {
@@ -11,34 +31,17 @@ pub const fn new_compatibility_date(year: i32, month: u32, day: u32) -> Compatib
1131
})
1232
}
1333

14-
/// Resolve `CompatibilityConfig` which is not part of metadata. Hence we resolve/build
15-
/// it separately.
16-
pub fn resolve_compatibility_config(
17-
raw_compatibility_config: &str,
18-
) -> Result<CompatibilityConfig, anyhow::Error> {
19-
Ok(open_dds::traits::OpenDd::deserialize(
20-
serde_json::from_str(raw_compatibility_config)?,
21-
)?)
22-
}
23-
24-
// given compatibility config, work out which warnings becomes errors using the date
25-
pub fn config_to_metadata_resolve(compat_config: &Option<CompatibilityConfig>) -> WarningsToRaise {
26-
match compat_config {
27-
Some(CompatibilityConfig::V1(compat_config_v1)) => {
28-
warnings_to_raise_from_date(&compat_config_v1.date)
29-
}
30-
Some(CompatibilityConfig::V2(compat_config_v2)) => {
31-
warnings_to_raise_from_date(&compat_config_v2.date)
32-
}
33-
34-
None => WarningsToRaise::default(),
35-
}
36-
}
37-
38-
// note we have no warnings to raise yet, so this is a no-op whilst we get the plumbing sorted
39-
fn warnings_to_raise_from_date(_date: &CompatibilityDate) -> WarningsToRaise {
40-
WarningsToRaise {
41-
// some_boolean_option: date >= &new_compatibility_date(2024, 1, 1)
42-
43-
}
34+
#[derive(Debug, PartialEq, thiserror::Error)]
35+
pub enum CompatibilityError {
36+
#[error("no compatibility config found")]
37+
NoCompatibilityConfigFound,
38+
#[error("duplicate compatibility config found")]
39+
DuplicateCompatibilityConfig,
40+
#[error("compatibility date {specified} is too old, oldest supported date is {oldest}")]
41+
DateTooOld {
42+
specified: CompatibilityDate,
43+
oldest: CompatibilityDate,
44+
},
45+
#[error("compatibility date {0} is in the future")]
46+
DateInTheFuture(CompatibilityDate),
4447
}

v3/crates/compatibility/src/error.rs

-16
This file was deleted.

v3/crates/compatibility/src/lib.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,7 @@
11
mod compatibility_date;
22
pub use compatibility_date::CompatibilityDate;
33

4-
mod error;
5-
pub use error::CompatibilityError;
6-
7-
mod types;
8-
pub use types::{CompatibilityConfigV1, CompatibilityConfigV2};
9-
104
mod config;
115
pub use config::{
12-
config_to_metadata_resolve, new_compatibility_date, resolve_compatibility_config,
6+
new_compatibility_date, CompatibilityConfigV1, CompatibilityConfigV2, CompatibilityError,
137
};

v3/crates/compatibility/src/types.rs

-40
This file was deleted.

v3/crates/engine/Cargo.toml

-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ name = "execute"
2121
harness = false
2222

2323
[dependencies]
24-
compatibility = { path = "../compatibility" }
2524
execute = { path = "../execute" }
2625
hasura-authn-core = { path = "../auth/hasura-authn-core" }
2726
hasura-authn-jwt = { path = "../auth/hasura-authn-jwt" }

v3/crates/engine/bin/engine/main.rs

+7-29
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,6 @@ struct ServerOptions {
6666
/// The configuration file used for authentication.
6767
#[arg(long, value_name = "PATH", env = "AUTHN_CONFIG_PATH")]
6868
authn_config_path: PathBuf,
69-
/// The configuration file used for compatibility.
70-
#[arg(long, value_name = "PATH", env = "COMPATIBILITY_CONFIG_PATH")]
71-
compatibility_config_path: Option<PathBuf>,
7269
/// The host IP on which the server listens, defaulting to all IPv4 and IPv6 addresses.
7370
#[arg(long, value_name = "HOST", env = "HOST", default_value_t = net::IpAddr::V6(net::Ipv6Addr::UNSPECIFIED))]
7471
host: net::IpAddr,
@@ -198,8 +195,6 @@ async fn shutdown_signal() {
198195
enum StartupError {
199196
#[error("could not read the auth config - {0}")]
200197
ReadAuth(anyhow::Error),
201-
#[error("could not read the compatibility config - {0}")]
202-
ReadCompatibility(anyhow::Error),
203198
#[error("failed to build engine state - {0}")]
204199
ReadSchema(anyhow::Error),
205200
}
@@ -353,6 +348,11 @@ impl EngineRouter {
353348

354349
#[allow(clippy::print_stdout)]
355350
async fn start_engine(server: &ServerOptions) -> Result<(), StartupError> {
351+
let metadata_resolve_configuration = metadata_resolve::configuration::Configuration {
352+
allow_unknown_subgraphs: server.partial_supergraph,
353+
unstable_features: resolve_unstable_features(&server.unstable_features),
354+
};
355+
356356
let expose_internal_errors = if server.expose_internal_errors {
357357
execute::ExposeInternalErrors::Expose
358358
} else {
@@ -362,10 +362,8 @@ async fn start_engine(server: &ServerOptions) -> Result<(), StartupError> {
362362
let state = build_state(
363363
expose_internal_errors,
364364
&server.authn_config_path,
365-
&server.compatibility_config_path,
366365
&server.metadata_path,
367-
server.partial_supergraph,
368-
resolve_unstable_features(&server.unstable_features),
366+
metadata_resolve_configuration,
369367
)
370368
.map_err(StartupError::ReadSchema)?;
371369

@@ -750,34 +748,14 @@ fn print_warnings<T: Display>(warnings: Vec<T>) {
750748
fn build_state(
751749
expose_internal_errors: execute::ExposeInternalErrors,
752750
authn_config_path: &PathBuf,
753-
compatibility_config_path: &Option<PathBuf>,
754751
metadata_path: &PathBuf,
755-
allow_unknown_subgraphs: bool,
756-
unstable_features: metadata_resolve::configuration::UnstableFeatures,
752+
metadata_resolve_configuration: metadata_resolve::configuration::Configuration,
757753
) -> Result<Arc<EngineState>, anyhow::Error> {
758754
// Auth Config
759755
let raw_auth_config = std::fs::read_to_string(authn_config_path)?;
760756
let (auth_config, auth_warnings) =
761757
resolve_auth_config(&raw_auth_config).map_err(StartupError::ReadAuth)?;
762758

763-
// Compatibility Config
764-
let compatibility_config = match compatibility_config_path {
765-
Some(path) => {
766-
let raw_config = std::fs::read_to_string(path)?;
767-
let compatibility_config = compatibility::resolve_compatibility_config(&raw_config)
768-
.map_err(StartupError::ReadCompatibility)?;
769-
Some(compatibility_config)
770-
}
771-
None => None,
772-
};
773-
774-
// derive metadata resolve configuration using compatibility configuration
775-
let metadata_resolve_configuration = metadata_resolve::configuration::Configuration {
776-
allow_unknown_subgraphs,
777-
unstable_features,
778-
warnings_to_raise: compatibility::config_to_metadata_resolve(&compatibility_config),
779-
};
780-
781759
// Metadata
782760
let raw_metadata = std::fs::read_to_string(metadata_path)?;
783761
let metadata = open_dds::Metadata::from_json_str(&raw_metadata)?;

v3/crates/engine/tests/common.rs

-1
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,6 @@ pub(crate) fn test_metadata_resolve_configuration() -> metadata_resolve::configu
548548
enable_order_by_expressions: false,
549549
enable_ndc_v02_support: true,
550550
},
551-
warnings_to_raise: metadata_resolve::configuration::WarningsToRaise {},
552551
}
553552
}
554553

v3/crates/metadata-resolve/src/types/configuration.rs

-7
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
pub struct Configuration {
77
pub allow_unknown_subgraphs: bool,
88
pub unstable_features: UnstableFeatures,
9-
pub warnings_to_raise: WarningsToRaise,
109
}
1110

1211
/// internal feature flags used in metadata resolve steps
@@ -18,9 +17,3 @@ pub struct UnstableFeatures {
1817
pub enable_order_by_expressions: bool,
1918
pub enable_ndc_v02_support: bool,
2019
}
21-
22-
/// struct of warnings that we'd like to raise to errors, based on CompatibilityConfig
23-
///
24-
/// Deserialization is only intended to be used for testing and is not reliable.
25-
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, serde::Deserialize)]
26-
pub struct WarningsToRaise {}

v3/crates/metadata-resolve/tests/metadata_golden_tests.rs

-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ fn read_test_configuration(
9797
Ok(configuration::Configuration {
9898
allow_unknown_subgraphs: false,
9999
unstable_features,
100-
warnings_to_raise: configuration::WarningsToRaise {},
101100
})
102101
}
103102
}

v3/justfile

-3
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ run-local-with-shell:
3737
RUST_LOG=DEBUG cargo run --bin engine -- \
3838
--otlp-endpoint http://localhost:4317 \
3939
--authn-config-path static/auth/auth_config.json \
40-
--compatibility-config-path static/compatibility/compatibility_config.json \
4140
--metadata-path crates/engine/tests/schema.json \
4241
--expose-internal-errors | ts "engine: " &
4342
wait
@@ -83,7 +82,6 @@ watch: start-docker-test-deps start-docker-run-deps
8382
-x 'run --bin engine -- \
8483
--otlp-endpoint http://localhost:4317 \
8584
--authn-config-path static/auth/auth_config.json \
86-
--compatibility-config-path static/compatibility/compatibility_config.json \
8785
--metadata-path crates/engine/tests/schema.json \
8886
--expose-internal-errors'
8987

@@ -140,7 +138,6 @@ run: start-docker-test-deps start-docker-run-deps
140138
RUST_LOG=DEBUG cargo run --bin engine -- \
141139
--otlp-endpoint http://localhost:4317 \
142140
--authn-config-path static/auth/auth_config.json \
143-
--compatibility-config-path static/compatibility/compatibility_config.json \
144141
--metadata-path crates/engine/tests/schema.json \
145142
--expose-internal-errors
146143

v3/static/compatibility/compatibility_config.json

-6
This file was deleted.

0 commit comments

Comments
 (0)