Skip to content

chore: remove Iceberg destination support - #1159

Open
farazdagi wants to merge 7 commits into
mainfrom
chore/remove-iceberg
Open

farazdagi wants to merge 7 commits into
mainfrom
chore/remove-iceberg

Conversation

@farazdagi

@farazdagi farazdagi commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Do not merge before the November 9, 2026 cutoff. Confirm that no Iceberg pipelines are running and that the internal shutdown is complete.

Remove the Iceberg destination, configuration variants, feature flags, and replicator dispatch, along with its tests, dependencies, local catalog/MinIO setup, and documentation. Add coverage rejecting retired configurations.

@farazdagi farazdagi self-assigned this Oct 23, 2026
@farazdagi
farazdagi force-pushed the chore/remove-iceberg branch from 0cb7b27 to fe86c2f Compare October 23, 2026 11:56
@farazdagi
farazdagi force-pushed the chore/remove-iceberg branch from 3754fd3 to 298a7a9 Compare October 23, 2026 12:56
@farazdagi
farazdagi marked this pull request as ready for review October 23, 2026 13:27
@farazdagi
farazdagi requested a review from a team as a code owner October 23, 2026 13:27

@iambriccardo iambriccardo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Feels amazing.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment on lines +1186 to +1196
/// Unsupported destination configurations must fail before a pipeline can
/// start.
#[test]
fn unsupported_destination_configs_are_rejected() {
let value = serde_json::json!({ "unsupported": {} });
let error = serde_json::from_value::<DestinationConfig>(value.clone()).unwrap_err();
assert!(error.to_string().contains("unknown variant `unsupported`"));
let error = serde_json::from_value::<DestinationConfigWithoutSecrets>(value).unwrap_err();
assert!(error.to_string().contains("unknown variant `unsupported`"));
let error = serde_json::from_str::<DestinationKind>("\"unsupported\"").unwrap_err();
assert!(error.to_string().contains("unknown variant `unsupported`"));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 nit (optional): The new rejection test never tries the retired iceberg key, so it does not cover the configurations this PR retires. It only deserializes a made-up unsupported variant, which was rejected before this change too. Fix: also assert that {"iceberg": {...}} (both rest and supabase shapes) fails for DestinationConfig and DestinationConfigWithoutSecrets, and that "iceberg" fails for DestinationKind.

Why this was flagged

The test unsupported_destination_configs_are_rejected at crates/etl-config/src/shared/destination.rs:1186-1196 builds serde_json::json!({ "unsupported": {} }) and "unsupported". No unsupported variant existed on the base branch either, so the test passes identically before and after the change. It says nothing about the retired iceberg key. If a future change re-adds an Iceberg variant or a serde alias, stored destination: iceberg: ... configs would deserialize again and this test would not catch it. The PR description promises coverage that rejects retired configurations, and this test does not provide it.

Verification: The test at crates/etl-config/src/shared/destination.rs:1186-1197 only deserializes unsupported inputs, which the base also rejected, so it passes the same way before and after this change. No test under crates/ checks that "iceberg" is now rejected, so a later change that brings back an Iceberg variant or serde alias would not be caught.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants