You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "Mitshah2406 (via GitHub)" <gi...@apache.org> on 2026/10/10 12:07:03 UTC
[PR] fix(connectors): exclude key and version from connector env mappings [iggy]
Mitshah2406 opened a new pull request, #4486:
URL: https://github.com/apache/iggy/pull/4486
Fixes #4396.
Overriding KEY renames a connector after its state path was derived, so it starts without its saved state. VERSION is identity rather than a runtime override for the same reason.
Marks both fields #[config_env(skip)] on sink and source configs so the names are neither parsed nor advertised, and drops the listing filter this makes redundant. Adds a unit test pinning KEY/VERSION out of the mappings and templates for both types.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): exclude key and version from connector env mappings [iggy]
Posted by "Mitshah2406 (via GitHub)" <gi...@apache.org>.
Mitshah2406 commented on PR #4486:
URL: https://github.com/apache/iggy/pull/4486#issuecomment-6110412763
/ready
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): exclude key and version from connector env mappings [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio merged PR #4486:
URL: https://github.com/apache/iggy/pull/4486
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): exclude key and version from connector env mappings [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on PR #4486:
URL: https://github.com/apache/iggy/pull/4486#issuecomment-6098312533
/skill team-review-slim
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): exclude key and version from connector env mappings [iggy]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #4486:
URL: https://github.com/apache/iggy/pull/4486#discussion_r4237894070
##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -176,8 +176,11 @@ impl CreateSinkConfig {
#[derive(Debug, Default, Clone, Serialize, Deserialize, ConfigEnv)]
pub struct SinkConfig {
+ // Identity comes from the file: an env KEY would orphan saved state.
+ #[config_env(skip)]
Review Comment:
warning: `IGGY_CONNECTORS_..._KEY` and `..._VERSION` now reach the unknown-variable scan, which makes a debug build panic at `typed_env_provider.rs:323` and warns with a field-name remedy in release. Treat both names as known-but-ignored and log that identity comes from the config file. Also at lines 183, 236, 239.
##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -176,8 +176,11 @@ impl CreateSinkConfig {
#[derive(Debug, Default, Clone, Serialize, Deserialize, ConfigEnv)]
pub struct SinkConfig {
+ // Identity comes from the file: an env KEY would orphan saved state.
Review Comment:
nit: The comment claims an env `KEY` orphans saved state, but only sources keep state. For a sink, an env key moves the consumer group instead. Reword the sink comment to name the identity change instead of state loss.
##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -488,6 +494,36 @@ impl ConnectorsConfig {
#[cfg(test)]
mod tests {
use super::*;
+ use configs::ConfigEnvMappings;
+
+ #[test]
+ fn given_sink_and_source_configs_when_mapped_should_exclude_key_and_version() {
+ for (name, mappings, templates) in [
+ (
+ "sink",
+ SinkConfig::env_mappings(),
+ SinkConfig::env_templates(),
+ ),
+ (
+ "source",
+ SourceConfig::env_mappings(),
+ SourceConfig::env_templates(),
+ ),
+ ] {
+ assert!(
Review Comment:
nit: Both assertions are `all()`-quantified, so an empty mapping list passes this test. Assert that a known name such as `ENABLED` is present before the exclusion checks.
##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -488,6 +494,36 @@ impl ConnectorsConfig {
#[cfg(test)]
mod tests {
use super::*;
+ use configs::ConfigEnvMappings;
+
+ #[test]
+ fn given_sink_and_source_configs_when_mapped_should_exclude_key_and_version() {
+ for (name, mappings, templates) in [
+ (
+ "sink",
+ SinkConfig::env_mappings(),
+ SinkConfig::env_templates(),
+ ),
+ (
+ "source",
+ SourceConfig::env_mappings(),
+ SourceConfig::env_templates(),
+ ),
+ ] {
+ assert!(
+ mappings
+ .iter()
+ .all(|mapping| !matches!(mapping.env_name, "KEY" | "VERSION")),
+ "{name} maps key or version from the environment"
+ );
+ assert!(
Review Comment:
simplification: `env_mappings()` only expands `env_templates()`, so this assertion and the one above it check the same absence twice. Keep the mappings assertion and delete the template block, which the listing test in `core/integration/tests/config_env_listing/mod.rs` already covers.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): exclude key and version from connector env mappings [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on PR #4486:
URL: https://github.com/apache/iggy/pull/4486#issuecomment-6124684303
please fix above comment, after that I think we can merge.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): exclude key and version from connector env mappings [iggy]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4486:
URL: https://github.com/apache/iggy/pull/4486#issuecomment-6097317993
<!-- iggy-pr-triage-welcome -->
Thanks for the PR. It is labeled `S-waiting-on-review` and queued for review.
Slash commands (own line, regular comment) move it around the queue:
- `/ready` - back to `S-waiting-on-review` after addressing feedback
- `/author` - flip to `S-waiting-on-author` while you finish changes
- `/request-review @user-or-team` - request a reviewer
- `/pin` - exempt the PR from the stale bot, `/unpin` to undo
See [CONTRIBUTING.md](https://github.com/apache/iggy/blob/master/CONTRIBUTING.md#pr-triage-commands) for details.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): exclude key and version from connector env mappings [iggy]
Posted by "Mitshah2406 (via GitHub)" <gi...@apache.org>.
Mitshah2406 commented on PR #4486:
URL: https://github.com/apache/iggy/pull/4486#issuecomment-6240619109
/ready
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): exclude key and version from connector env mappings [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on code in PR #4486:
URL: https://github.com/apache/iggy/pull/4486#discussion_r4248176496
##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -229,8 +232,11 @@ impl CreateSourceConfig {
#[derive(Debug, Default, Clone, Serialize, Deserialize, ConfigEnv)]
pub struct SourceConfig {
+ // Identity comes from the file: an env KEY would orphan saved state.
+ #[config_env(skip)]
Review Comment:
warning: if a retired `KEY` override differs from the file key, an upgrade silently selects a different checkpoint and starts without the saved state. reject conflicting overrides with migration instructions.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): exclude key and version from connector env mappings [iggy]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #4486:
URL: https://github.com/apache/iggy/pull/4486#issuecomment-6098096462
## [Codecov](https://app.codecov.io/gh/apache/iggy/pull/4486?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `90.90909%` with `2 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 72.61%. Comparing base ([`8efc3e1`](https://app.codecov.io/gh/apache/iggy/commit/8efc3e1e651d92afa1478ef2819dae916c94ad98?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1e31041`](https://app.codecov.io/gh/apache/iggy/commit/1e310415e54e0007e0e348c2defdb60df19a8886?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/4486?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [core/connectors/runtime/src/configs/connectors.rs](https://app.codecov.io/gh/apache/iggy/pull/4486?src=pr&el=tree&filepath=core%2Fconnectors%2Fruntime%2Fsrc%2Fconfigs%2Fconnectors.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3J1bnRpbWUvc3JjL2NvbmZpZ3MvY29ubmVjdG9ycy5ycw==) | 90.90% | [0 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/iggy/pull/4486?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #4486 +/- ##
=============================================
- Coverage 88.40% 72.61% -15.80%
Complexity 1608 1608
=============================================
Files 1294 1241 -53
Lines 249269 153430 -95839
Branches 209775 113936 -95839
=============================================
- Hits 220374 111414 -108960
- Misses 23608 36831 +13223
+ Partials 5287 5185 -102
```
| [Components](https://app.codecov.io/gh/apache/iggy/pull/4486/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [Rust Core](https://app.codecov.io/gh/apache/iggy/pull/4486/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.72% <90.90%> (-20.82%)` | :arrow_down: |
| [Java SDK](https://app.codecov.io/gh/apache/iggy/pull/4486/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.82% <ø> (ø)` | |
| [C# SDK](https://app.codecov.io/gh/apache/iggy/pull/4486/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.91% <ø> (ø)` | |
| [Python SDK](https://app.codecov.io/gh/apache/iggy/pull/4486/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `91.07% <ø> (ø)` | |
| [PHP SDK](https://app.codecov.io/gh/apache/iggy/pull/4486/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.67% <ø> (ø)` | |
| [Node SDK](https://app.codecov.io/gh/apache/iggy/pull/4486/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `96.59% <ø> (ø)` | |
| [Go SDK](https://app.codecov.io/gh/apache/iggy/pull/4486/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.29% <ø> (ø)` | |
| [C++ SDK](https://app.codecov.io/gh/apache/iggy/pull/4486/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.37% <ø> (ø)` | |
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/4486?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [core/connectors/runtime/src/main.rs](https://app.codecov.io/gh/apache/iggy/pull/4486?src=pr&el=tree&filepath=core%2Fconnectors%2Fruntime%2Fsrc%2Fmain.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3J1bnRpbWUvc3JjL21haW4ucnM=) | `89.14% <ø> (+0.53%)` | :arrow_up: |
| [core/connectors/runtime/src/configs/connectors.rs](https://app.codecov.io/gh/apache/iggy/pull/4486?src=pr&el=tree&filepath=core%2Fconnectors%2Fruntime%2Fsrc%2Fconfigs%2Fconnectors.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3J1bnRpbWUvc3JjL2NvbmZpZ3MvY29ubmVjdG9ycy5ycw==) | `67.75% <90.90%> (+2.65%)` | :arrow_up: |
... and [494 files with indirect coverage changes](https://app.codecov.io/gh/apache/iggy/pull/4486/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(connectors): exclude key and version from connector env mappings [iggy]
Posted by "Mitshah2406 (via GitHub)" <gi...@apache.org>.
Mitshah2406 commented on code in PR #4486:
URL: https://github.com/apache/iggy/pull/4486#discussion_r4322905798
##########
core/connectors/runtime/src/configs/connectors.rs:
##########
@@ -229,8 +232,11 @@ impl CreateSourceConfig {
#[derive(Debug, Default, Clone, Serialize, Deserialize, ConfigEnv)]
pub struct SourceConfig {
+ // Identity comes from the file: an env KEY would orphan saved state.
+ #[config_env(skip)]
Review Comment:
Resolving the comment with this commit - https://github.com/apache/iggy/pull/4486/changes/3d83137439088d202f6bda4a773f78389ea9c19c
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org