hubcio commented on code in PR #4350:
URL: https://github.com/apache/iggy/pull/4350#discussion_r4158611332
##########
core/configs/src/configs_impl/env_mapping.rs:
##########
@@ -59,3 +74,120 @@ pub trait ConfigEnvMappings {
.collect()
}
}
+
+impl EnvVarTemplate {
+ /// Expands this template into every concrete env-var name it represents,
+ /// substituting each `<N>` placeholder left-to-right with `0..limit`.
+ /// A leaf template (`max_elements: &[]`) expands to itself.
+ pub fn expand_names(&self) -> Vec<String> {
+ if self.max_elements.is_empty() {
+ return vec![self.env_name.to_string()];
+ }
+
+ let mut results = vec![self.env_name.to_string()];
+
+ for &limit in self.max_elements {
+ let mut next = Vec::new();
+ for name in results {
+ for i in 0..limit {
+ next.push(name.replacen("<N>", &i.to_string(), 1));
+ }
+ }
+ results = next;
+ }
+
+ results
+ }
+}
+
+#[cfg(test)]
+mod consistency_tests {
+ use super::*;
+ use std::collections::HashSet;
+ use crate::server::ServerConfig;
+ use crate::cluster::ClusterConfig;
+ use crate::McpServerConfig;
Review Comment:
critical: `configs` cannot import `McpServerConfig` or the connectors config
types, since those binaries depend on `configs` - the test target fails with
E0432, so no `configs` test runs. move these four parity tests next to their
types.
##########
core/connectors/runtime/src/main.rs:
##########
@@ -117,10 +132,50 @@ fn print_ascii_art(text: &str) {
println!("{}", figure.unwrap());
}
-#[tokio::main]
-async fn main() -> Result<(), RuntimeError> {
+fn main() -> Result<(), RuntimeError> {
+ let args = Args::parse();
+ if args.list_config_env_vars {
+ print_config_env_vars()?;
+ return Ok(());
+ }
capture_allowed_cpus();
- Args::parse();
+ Builder::new_multi_thread()
+ .enable_all()
+ .build()?
+ .block_on(run())
+}
+
+fn print_config_env_vars() -> std::io::Result<()> {
+ let prefix = ConnectorsRuntimeConfig::ENV_PREFIX;
+ let sink_source_templates: Vec<String> = [
+ ("SINK", SinkConfig::env_templates()),
+ ("SOURCE", SourceConfig::env_templates()),
+ ]
+ .iter()
+ .flat_map(|(kind, templates)| {
+ templates
+ .iter()
+ .map(move |template| format!("{prefix}{kind}_<KEY>_{}",
template.env_name))
+ .chain(
+ [
Review Comment:
warning: clippy rejects the `.into_iter()` on this array as
`useless_conversion` - `chain` takes any `IntoIterator`, so pass it directly.
also at line 173.
##########
core/connectors/runtime/src/main.rs:
##########
@@ -117,10 +132,50 @@ fn print_ascii_art(text: &str) {
println!("{}", figure.unwrap());
}
-#[tokio::main]
-async fn main() -> Result<(), RuntimeError> {
+fn main() -> Result<(), RuntimeError> {
+ let args = Args::parse();
+ if args.list_config_env_vars {
+ print_config_env_vars()?;
+ return Ok(());
+ }
capture_allowed_cpus();
- Args::parse();
+ Builder::new_multi_thread()
+ .enable_all()
+ .build()?
+ .block_on(run())
+}
+
+fn print_config_env_vars() -> std::io::Result<()> {
+ let prefix = ConnectorsRuntimeConfig::ENV_PREFIX;
+ let sink_source_templates: Vec<String> = [
+ ("SINK", SinkConfig::env_templates()),
+ ("SOURCE", SourceConfig::env_templates()),
+ ]
+ .iter()
+ .flat_map(|(kind, templates)| {
+ templates
+ .iter()
+ .map(move |template| format!("{prefix}{kind}_<KEY>_{}",
template.env_name))
Review Comment:
warning: the listing now advertises `IGGY_CONNECTORS_SOURCE_<KEY>_KEY`, and
setting it renames the source, so it starts without its saved state. not new in
this PR - worth an issue to mark `key` and `version` `#[config_env(skip)]` on
sinks and sources.
##########
core/integration/tests/config_env_listing/mod.rs:
##########
@@ -0,0 +1,219 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use assert_cmd::Command;
+use std::collections::HashSet;
+use std::time::Duration;
+
+const LIST_ENV_VARS_TIMEOUT: Duration = Duration::from_secs(5);
+
+#[test]
+fn config_env_listing_exits_before_startup_for_each_binary() {
+ for (binary, config_env, dotenv_env) in [
+ ("iggy-server", "IGGY_CONFIG_PATH", "IGGY_ENV_PATH"),
+ (
+ "iggy-connectors",
+ "IGGY_CONNECTORS_CONFIG_PATH",
+ "IGGY_CONNECTORS_ENV_PATH",
+ ),
+ ("iggy-mcp", "IGGY_MCP_CONFIG_PATH", "IGGY_MCP_ENV_PATH"),
+ ] {
+ let directory = tempfile::tempdir().expect("temporary directory");
+ let output = Command::cargo_bin(binary)
+ .expect("binary should be built")
+ .current_dir(directory.path())
+ .arg("--list-config-env-vars")
+ .env(config_env, directory.path().join("missing-config.toml"))
+ .env(dotenv_env, directory.path().join("missing.env"))
+ .timeout(LIST_ENV_VARS_TIMEOUT)
+ .output()
+ .expect("listing command should run");
+
+ assert!(output.status.success(), "{binary} failed");
+ assert!(output.stderr.is_empty(), "{binary} wrote to stderr");
+
+ let stdout = String::from_utf8(output.stdout).expect("UTF-8 output");
+ let names: Vec<_> = stdout.lines().collect();
+ assert!(!names.is_empty(), "{binary} returned no variables");
+ assert!(
+ names.windows(2).all(|pair| pair[0] < pair[1]),
+ "{binary} output must be sorted and deduplicated"
+ );
+ assert!(names.iter().all(|name| name.starts_with("IGGY_")));
Review Comment:
warning: this only checks the `IGGY_` prefix, so no test pins the #4322
exclusions. assert the server output has none of `SERVER_SCAN_ONLY_ENV_VARS` or
the sibling prefixes, and connectors and mcp names use their own prefix or
`IGGY_DISPLAY_CONFIG`.
##########
core/configs/src/server_config/server.rs:
##########
@@ -47,6 +47,34 @@ pub use crate::common::server::{
TelemetryConfig, TelemetryLogsConfig, TelemetryTracesConfig,
TelemetryTransport,
};
+/// Vars safe to advertise to end users via `--list-config-env-vars`.
+pub const SERVER_RUNTIME_ENV_VARS: &[&str] = &[
+ "IGGY_CONFIG_PATH",
+ "IGGY_DISPLAY_CONFIG",
+ "IGGY_ENV_PATH",
+ "IGGY_ROOT_PASSWORD",
+ "IGGY_ROOT_USERNAME",
+ "IGGY_SHARD_EVENT_INTERVAL",
+ "IGGY_SHARD_RUNTIME_CAPACITY",
+];
+
+/// Test/CI-only vars: never advertised, but still non-config (suppress
Review Comment:
nit: `IGGY_HOME`, `IGGY_USERNAME` and `IGGY_PASSWORD` are not test-only -
the `iggy` CLI reads them, and the server reads none of the three. describe
them as sibling-binary vars here and at line 73.
##########
core/ai/mcp/src/error.rs:
##########
@@ -45,4 +45,6 @@ pub enum McpRuntimeError {
TokenFileReadError(String, String),
#[error("Token file is empty: {0}")]
TokenFileEmpty(String),
+ #[error("I/O error: {0}")]
+ Io(#[from] std::io::Error),
Review Comment:
nit: same as the server - this context-free `#[from] std::io::Error` sits
next to `RuntimeCreation` and `SignalRegistration`, which name their operation.
use a named `#[source]` variant and `map_err` at main.rs:57.
##########
core/configs/src/configs_impl/env_mapping.rs:
##########
@@ -59,3 +74,120 @@ pub trait ConfigEnvMappings {
.collect()
}
}
+
+impl EnvVarTemplate {
+ /// Expands this template into every concrete env-var name it represents,
+ /// substituting each `<N>` placeholder left-to-right with `0..limit`.
+ /// A leaf template (`max_elements: &[]`) expands to itself.
+ pub fn expand_names(&self) -> Vec<String> {
+ if self.max_elements.is_empty() {
+ return vec![self.env_name.to_string()];
+ }
+
+ let mut results = vec![self.env_name.to_string()];
+
+ for &limit in self.max_elements {
+ let mut next = Vec::new();
+ for name in results {
+ for i in 0..limit {
+ next.push(name.replacen("<N>", &i.to_string(), 1));
+ }
+ }
+ results = next;
+ }
+
+ results
+ }
+}
+
+#[cfg(test)]
+mod consistency_tests {
+ use super::*;
+ use std::collections::HashSet;
+ use crate::server::ServerConfig;
+ use crate::cluster::ClusterConfig;
+ use crate::McpServerConfig;
+ use configs::runtime::ConnectorsRuntimeConfig;
+ use configs::connectors::{SinkConfig, SourceConfig};
+
+ #[test]
+ fn server_config_templates_and_mappings_align() {
Review Comment:
simplification: the six tests share one body, and the `ClusterConfig` one
repeats what the `ServerConfig` test covers. drop it, and after the move use
one generic helper for the three connectors types.
##########
core/configs/src/server_config/server.rs:
##########
@@ -47,6 +47,34 @@ pub use crate::common::server::{
TelemetryConfig, TelemetryLogsConfig, TelemetryTracesConfig,
TelemetryTransport,
};
+/// Vars safe to advertise to end users via `--list-config-env-vars`.
+pub const SERVER_RUNTIME_ENV_VARS: &[&str] = &[
Review Comment:
simplification: `SERVER_RUNTIME_ENV_VARS` retypes 7 names of
`SERVER_PROCESS_ENV_VARS`, and a test keeps them in sync. print
`SERVER_PROCESS_ENV_VARS` minus `SERVER_SCAN_ONLY_ENV_VARS`, and drop this list
and its test.
##########
core/connectors/runtime/src/main.rs:
##########
@@ -117,10 +132,50 @@ fn print_ascii_art(text: &str) {
println!("{}", figure.unwrap());
}
-#[tokio::main]
-async fn main() -> Result<(), RuntimeError> {
+fn main() -> Result<(), RuntimeError> {
+ let args = Args::parse();
+ if args.list_config_env_vars {
+ print_config_env_vars()?;
+ return Ok(());
+ }
capture_allowed_cpus();
- Args::parse();
+ Builder::new_multi_thread()
+ .enable_all()
+ .build()?
+ .block_on(run())
+}
+
+fn print_config_env_vars() -> std::io::Result<()> {
+ let prefix = ConnectorsRuntimeConfig::ENV_PREFIX;
+ let sink_source_templates: Vec<String> = [
+ ("SINK", SinkConfig::env_templates()),
+ ("SOURCE", SourceConfig::env_templates()),
+ ]
+ .iter()
+ .flat_map(|(kind, templates)| {
+ templates
+ .iter()
+ .map(move |template| format!("{prefix}{kind}_<KEY>_{}",
template.env_name))
Review Comment:
simplification: #4322 asks to reuse the provider naming rules, but this
rebuilds the `<KEY>` prefix and `PLUGIN_CONFIG_` rule that
local_provider.rs:351 and :825 build. move the rule into one `pub(crate)` fn
and call it from both.
##########
core/integration/tests/config_env_listing/mod.rs:
##########
@@ -0,0 +1,219 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use assert_cmd::Command;
+use std::collections::HashSet;
+use std::time::Duration;
+
+const LIST_ENV_VARS_TIMEOUT: Duration = Duration::from_secs(5);
+
+#[test]
+fn config_env_listing_exits_before_startup_for_each_binary() {
+ for (binary, config_env, dotenv_env) in [
+ ("iggy-server", "IGGY_CONFIG_PATH", "IGGY_ENV_PATH"),
+ (
+ "iggy-connectors",
+ "IGGY_CONNECTORS_CONFIG_PATH",
+ "IGGY_CONNECTORS_ENV_PATH",
+ ),
+ ("iggy-mcp", "IGGY_MCP_CONFIG_PATH", "IGGY_MCP_ENV_PATH"),
+ ] {
+ let directory = tempfile::tempdir().expect("temporary directory");
+ let output = Command::cargo_bin(binary)
+ .expect("binary should be built")
+ .current_dir(directory.path())
+ .arg("--list-config-env-vars")
+ .env(config_env, directory.path().join("missing-config.toml"))
+ .env(dotenv_env, directory.path().join("missing.env"))
+ .timeout(LIST_ENV_VARS_TIMEOUT)
+ .output()
+ .expect("listing command should run");
+
+ assert!(output.status.success(), "{binary} failed");
+ assert!(output.stderr.is_empty(), "{binary} wrote to stderr");
+
+ let stdout = String::from_utf8(output.stdout).expect("UTF-8 output");
+ let names: Vec<_> = stdout.lines().collect();
+ assert!(!names.is_empty(), "{binary} returned no variables");
+ assert!(
+ names.windows(2).all(|pair| pair[0] < pair[1]),
+ "{binary} output must be sorted and deduplicated"
+ );
+ assert!(names.iter().all(|name| name.starts_with("IGGY_")));
+ }
+}
+
+#[test]
+fn config_env_listing_includes_vector_index_templates() {
+ // Verify that vector fields are correctly represented with <N> placeholder
+ let directory = tempfile::tempdir().expect("temporary directory");
+ let output = Command::cargo_bin("iggy-server")
Review Comment:
simplification: the six `Command::cargo_bin` builders repeat - one helper
that takes the binary, extra args and env pairs and runs in a fresh tempdir
saves about 12 lines.
##########
core/server/src/server_error.rs:
##########
@@ -313,6 +313,8 @@ pub enum ServerError {
/// as clean.
#[error("server shut down after a panic: {description}")]
Panicked { description: String },
+ #[error("I/O error: {0}")]
+ Io(#[from] std::io::Error),
Review Comment:
warning: this `#[from]` lets any later `?` on an io error compile into a
context-free `Io`, and a failed listing prints just `Error: Io(Os { .. })`. use
a named variant like `ListConfigEnvVars(#[source] std::io::Error)` with
`map_err` at main.rs:46.
##########
core/configs/src/server_config/server.rs:
##########
@@ -47,6 +47,34 @@ pub use crate::common::server::{
TelemetryConfig, TelemetryLogsConfig, TelemetryTracesConfig,
TelemetryTransport,
};
+/// Vars safe to advertise to end users via `--list-config-env-vars`.
+pub const SERVER_RUNTIME_ENV_VARS: &[&str] = &[
+ "IGGY_CONFIG_PATH",
+ "IGGY_DISPLAY_CONFIG",
+ "IGGY_ENV_PATH",
+ "IGGY_ROOT_PASSWORD",
+ "IGGY_ROOT_USERNAME",
+ "IGGY_SHARD_EVENT_INTERVAL",
+ "IGGY_SHARD_RUNTIME_CAPACITY",
+];
+
+/// Test/CI-only vars: never advertised, but still non-config (suppress
+/// "unknown env var" warnings the same as `SERVER_RUNTIME_ENV_VARS` does).
Review Comment:
nit: `SERVER_RUNTIME_ENV_VARS` does not suppress anything - only
`SERVER_PROCESS_ENV_VARS` reaches the unknown-name scan near line 313. reword
this, and drop "reads directly" at line 73.
##########
core/server/README.md:
##########
@@ -27,7 +27,7 @@ To run one node of a cluster, pass its replica ID from the
`cluster.nodes` roste
cargo run --bin iggy-server --release -- --replica-id 0
```
-`--replica-id` is the only command line argument; everything else is
configuration.
+Command line arguments include `--replica-id`, `--fresh` (`-f`),
`--with-default-root-credentials`, and `--list-config-env-vars`; everything
else is configuration.
Review Comment:
nit: #4322 also asks for the configuration docs - please document the flag
and its `<N>`, `<KEY>` and `<FIELD>` placeholders on the website configuration
pages too.
##########
core/server/src/args.rs:
##########
@@ -87,6 +87,21 @@ For more information, visit:
https://iggy.apache.org/docs/introduction/getting-s
// variable names and paths must stay unquoted rather than wear rustdoc
backticks.
#[allow(clippy::doc_markdown)]
pub struct Args {
+ #[arg(long, long_help = r#"Print supported configuration environment
variables and exit.
Review Comment:
nit: with only `long_help`, clap prints this whole block for `-h` too. add a
one-line `help` and keep the details in `long_help`.
also at core/connectors/runtime/src/main.rs:73.
##########
core/server/src/main.rs:
##########
@@ -119,7 +123,7 @@ fn main() -> Result<(), ServerError> {
if let Err(error) = &joined
&& descriptors_exhausted()
{
- // `fatal` skips destructors, and the log appenders flush on drop.
+ // `fatal` skips destructors, and the log flush must run
Review Comment:
nit: this comment lost its reason - restore "`fatal` skips destructors, and
the log appenders flush on drop." from master.
##########
core/connectors/runtime/src/main.rs:
##########
@@ -117,10 +132,50 @@ fn print_ascii_art(text: &str) {
println!("{}", figure.unwrap());
}
-#[tokio::main]
-async fn main() -> Result<(), RuntimeError> {
+fn main() -> Result<(), RuntimeError> {
+ let args = Args::parse();
+ if args.list_config_env_vars {
+ print_config_env_vars()?;
+ return Ok(());
+ }
capture_allowed_cpus();
- Args::parse();
+ Builder::new_multi_thread()
+ .enable_all()
+ .build()?
+ .block_on(run())
+}
+
+fn print_config_env_vars() -> std::io::Result<()> {
+ let prefix = ConnectorsRuntimeConfig::ENV_PREFIX;
+ let sink_source_templates: Vec<String> = [
+ ("SINK", SinkConfig::env_templates()),
+ ("SOURCE", SourceConfig::env_templates()),
+ ]
+ .iter()
+ .flat_map(|(kind, templates)| {
+ templates
+ .iter()
+ .map(move |template| format!("{prefix}{kind}_<KEY>_{}",
template.env_name))
+ .chain(
+ [
+ format!("{prefix}{kind}_<KEY>_ENABLED"),
Review Comment:
simplification: `enabled` already derives `<KEY>_ENABLED`, and only the sort
and dedup hide the duplicate. drop this entry, chain `std::iter::once` for
`PLUGIN_CONFIG_<FIELD>`, take the array by value and skip the `Vec` - that also
clears both clippy errors.
##########
core/connectors/runtime/src/main.rs:
##########
@@ -67,7 +69,20 @@ static GLOBAL: MiMalloc = MiMalloc;
#[derive(Parser, Debug)]
#[command(author = "Apache Iggy", version)]
-struct Args {}
+struct Args {
+ #[arg(long, long_help = r#"Print supported configuration environment
variables and exit.
+
+Lists all supported IGGY_* environment variable names and templates,
+sorted and deduplicated. Template syntax:
+- <N> represents vector indices
Review Comment:
nit: #4322 asks for per-field index limits - every `<N>` here comes from
`streams`, which takes 0-255, so say that like args.rs:94 does for the server.
##########
core/integration/tests/config_env_listing/mod.rs:
##########
@@ -0,0 +1,219 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use assert_cmd::Command;
+use std::collections::HashSet;
+use std::time::Duration;
+
+const LIST_ENV_VARS_TIMEOUT: Duration = Duration::from_secs(5);
+
+#[test]
+fn config_env_listing_exits_before_startup_for_each_binary() {
+ for (binary, config_env, dotenv_env) in [
+ ("iggy-server", "IGGY_CONFIG_PATH", "IGGY_ENV_PATH"),
+ (
+ "iggy-connectors",
+ "IGGY_CONNECTORS_CONFIG_PATH",
+ "IGGY_CONNECTORS_ENV_PATH",
+ ),
+ ("iggy-mcp", "IGGY_MCP_CONFIG_PATH", "IGGY_MCP_ENV_PATH"),
+ ] {
+ let directory = tempfile::tempdir().expect("temporary directory");
+ let output = Command::cargo_bin(binary)
+ .expect("binary should be built")
+ .current_dir(directory.path())
+ .arg("--list-config-env-vars")
+ .env(config_env, directory.path().join("missing-config.toml"))
+ .env(dotenv_env, directory.path().join("missing.env"))
+ .timeout(LIST_ENV_VARS_TIMEOUT)
+ .output()
+ .expect("listing command should run");
+
+ assert!(output.status.success(), "{binary} failed");
Review Comment:
nit: on failure this prints only the binary name - add `output.status` and
the stderr text, so a red run says why.
also at lines 163, 167, 191.
##########
core/configs_derive/src/config_env.rs:
##########
@@ -359,6 +389,49 @@ fn generate_struct_impl(
}
};
+ let own_template_entries = mappings.iter().map(|mapping| {
Review Comment:
simplification: `env_mappings()` and `env_templates()` walk the same field
tree, and the parity tests exist only to keep them in step. generate templates
with `config_path` and `is_secret`, and expand them in `env_mappings()` - note
the `Vec` entries then change order.
##########
core/configs/src/configs_impl/env_listing.rs:
##########
@@ -0,0 +1,59 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::io::Write;
+
+/// Writes each name in `names`, sorted and deduplicated, to stdout — one
+/// call, one lock, one error policy. A closed stdout (e.g.
+/// `iggy-server --list-config-env-vars | head -1`) is expected, not a
+/// failure: on `BrokenPipe` this stops writing and returns `Ok(())`. Any
+/// other I/O error is real and is propagated so the caller exits non-zero.
+pub fn print_env_var_names<I, S>(names: I) -> std::io::Result<()>
Review Comment:
nit: this hardcodes `stdout()`, so no test reaches the `BrokenPipe` branch
or the error branch. take a `&mut impl Write` and unit-test both.
##########
core/configs/src/configs_impl/env_mapping.rs:
##########
@@ -59,3 +74,120 @@ pub trait ConfigEnvMappings {
.collect()
}
}
+
+impl EnvVarTemplate {
+ /// Expands this template into every concrete env-var name it represents,
+ /// substituting each `<N>` placeholder left-to-right with `0..limit`.
+ /// A leaf template (`max_elements: &[]`) expands to itself.
+ pub fn expand_names(&self) -> Vec<String> {
+ if self.max_elements.is_empty() {
Review Comment:
simplification: this early return is redundant - with an empty
`max_elements`, the loop below already returns `vec![env_name]`.
##########
core/configs/src/configs_impl/env_listing.rs:
##########
@@ -0,0 +1,59 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::io::Write;
+
+/// Writes each name in `names`, sorted and deduplicated, to stdout — one
+/// call, one lock, one error policy. A closed stdout (e.g.
+/// `iggy-server --list-config-env-vars | head -1`) is expected, not a
+/// failure: on `BrokenPipe` this stops writing and returns `Ok(())`. Any
+/// other I/O error is real and is propagated so the caller exits non-zero.
+pub fn print_env_var_names<I, S>(names: I) -> std::io::Result<()>
+where
+ I: IntoIterator<Item = S>,
+ S: Into<String>,
+{
+ let mut names: Vec<String> = names.into_iter().map(Into::into).collect();
+ names.sort_unstable();
+ names.dedup();
+
+ let stdout = std::io::stdout();
+ let mut lock = stdout.lock();
+ for name in names {
+ if let Err(err) = writeln!(lock, "{name}") {
+ return if err.kind() == std::io::ErrorKind::BrokenPipe {
+ Ok(())
+ } else {
+ Err(err)
+ };
+ }
+ }
+ Ok(())
+}
+
+/// Env vars `iggy-mcp --list-config-env-vars` advertises beyond the derived
+/// `McpServerConfig` templates.
+pub const MCP_RUNTIME_ENV_VARS: &[&str] =
Review Comment:
simplification: these names are typed again at each `env::var` call in the
mcp and connectors mains. give each name one constant that the list and the
read share.
##########
core/configs/src/configs_impl/env_listing.rs:
##########
@@ -0,0 +1,59 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::io::Write;
+
+/// Writes each name in `names`, sorted and deduplicated, to stdout — one
+/// call, one lock, one error policy. A closed stdout (e.g.
+/// `iggy-server --list-config-env-vars | head -1`) is expected, not a
+/// failure: on `BrokenPipe` this stops writing and returns `Ok(())`. Any
+/// other I/O error is real and is propagated so the caller exits non-zero.
+pub fn print_env_var_names<I, S>(names: I) -> std::io::Result<()>
+where
+ I: IntoIterator<Item = S>,
+ S: Into<String>,
+{
+ let mut names: Vec<String> = names.into_iter().map(Into::into).collect();
+ names.sort_unstable();
+ names.dedup();
+
+ let stdout = std::io::stdout();
+ let mut lock = stdout.lock();
+ for name in names {
+ if let Err(err) = writeln!(lock, "{name}") {
+ return if err.kind() == std::io::ErrorKind::BrokenPipe {
+ Ok(())
+ } else {
+ Err(err)
+ };
+ }
+ }
+ Ok(())
+}
+
+/// Env vars `iggy-mcp --list-config-env-vars` advertises beyond the derived
+/// `McpServerConfig` templates.
+pub const MCP_RUNTIME_ENV_VARS: &[&str] =
Review Comment:
simplification: these lists repeat the four path names of `IGNORED_ENV_VARS`
(typed_env_provider.rs:49). let the scan at typed_env_provider.rs:294 read
these lists, and delete `IGNORED_ENV_VARS`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
