Skip to content

feat(gooddata-pipelines): make workspace data filter fields optional#1609

Merged
benkeanna merged 1 commit into
masterfrom
aben/optional-wdf
May 13, 2026
Merged

feat(gooddata-pipelines): make workspace data filter fields optional#1609
benkeanna merged 1 commit into
masterfrom
aben/optional-wdf

Conversation

@benkeanna
Copy link
Copy Markdown
Contributor

@benkeanna benkeanna commented May 12, 2026

Summary

https://gooddata.atlassian.net/browse/MCMIC-2430

Makes workspace_data_filter_id and workspace_data_filter_column_name on CustomDatasetDefinition optional (str | None = None). Previously both were required, which forced callers to provide values even for datasets that don't participate in a workspace data filter — a shape the underlying gooddata_sdk already supports (workspace_data_filter_columns and workspace_data_filter_references on CatalogDeclarativeDataset are both list[...] | None = None).

Driven by a real need on the MIC BCA tooling: BCA datasets per spec don't have WDF, and the wrapper API was blocking that legitimate use case.

Backward compatibility

  • Existing callers passing both fields keep working byte-for-byte.
  • A check_wdf_pair validator enforces both-or-neither: setting only one of the two now raises a clear ValidationError instead of producing a malformed declarative dataset.
  • The processor only emits workspace_data_filter_columns / workspace_data_filter_references when both fields are non-None; otherwise the SDK receives None (which it accepts).

Test plan

  • pytest tests/ — 186 passing.
  • ruff check src/ tests/ — clean.
  • Validate end-to-end with a real workspace that has a dataset without WDF.

🤖 Generated with Claude Code

@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.01%. Comparing base (0c299a0) to head (a1194a0).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1609      +/-   ##
==========================================
+ Coverage   78.99%   79.01%   +0.02%     
==========================================
  Files         231      231              
  Lines       15603    15619      +16     
==========================================
+ Hits        12325    12341      +16     
  Misses       3278     3278              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines +67 to +69
Workspace data filter fields are optional. Both must be set together or
both left unset; when set, a single-column WDF binding is emitted on the
declarative dataset.
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.

this is obvious from the validator

Comment on lines +256 to +258
# Workspace data filter fields are optional and must be set together
# (validated on the input model). Emit columns/references only when
# both are provided.
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.

This is duplicating the docstring on the validator. Not sure if it is necessary here

Comment on lines +314 to +315
workspace_data_filter_columns=wdf_columns or None,
workspace_data_filter_references=wdf_references or None,
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.

What would happen if we passed the empty lists here?

@benkeanna benkeanna force-pushed the aben/optional-wdf branch 3 times, most recently from 111991b to 2d426cc Compare May 13, 2026 07:21
@benkeanna benkeanna marked this pull request as ready for review May 13, 2026 11:30
if wdf_id is not None:
# `check_wdf_pair` on the model guarantees the column name is
# set whenever the id is.
assert wdf_column_name is not None
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.

maybe you can add the wdf_column_name to the condition check above?

@benkeanna benkeanna force-pushed the aben/optional-wdf branch from 2d426cc to e8a1375 Compare May 13, 2026 12:40
janmatzek
janmatzek previously approved these changes May 13, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@benkeanna benkeanna force-pushed the aben/optional-wdf branch from dbd27bc to a1194a0 Compare May 13, 2026 13:58
@benkeanna benkeanna enabled auto-merge May 13, 2026 14:03
@benkeanna benkeanna merged commit c713a9a into master May 13, 2026
13 checks passed
@benkeanna benkeanna deleted the aben/optional-wdf branch May 13, 2026 14:11
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