mirror of
https://github.com/microsoft/fabric-cicd.git
synced 2026-11-04 12:02:01 +00:00
arhx91andshirasassoon
4bf12265eb
## Description With this PR we add an opt-in feature flag, `enable_strict_semantic_model_binding`. The flag enables a deployment to fail loudly when a semantic model connection binding fails. Note that default behavior does not change: as it stands fabric-cicd logs the failure and continues. As agreed in #1157 , neither do we roll anything back (since bindings runs after publishing semantic model definition). So, in summary, the flag gives us two things: 1. The deployment ends with an error, so the pipeline run is red. 2. In the standard publish order, item types after semantic models, such as reports, are not published on top of a model that has no working connection. Bulk publish is the exception. In this case all items are published before the binding runs. ### Behavior - `bind_semanticmodel_to_connection()` returns a message per binding that did not happen. All bindings are attempted first. A failure does not stop the other bindings. - If the flag is set and there is at least one failure, `SemanticModelPublisher.post_publish_all()` raises one `FailedPublishedItemStatusError` that lists every failure. The full list is in the exception message, so `str(error)` and `DeploymentResult.message` contain the model names and connection IDs for targeted debugging. #### What does and does not count as failure? Following are considered 'failures': - a connection ID that is not found in List Connections - a failed call to list the existing connections of the model - a `bindConnection` call that returns a status other than 200 - a `bindConnection` call that raises These cases do not count as failures (they stay at DEBUG level): - semantic models excluded by `items_to_include` - semantic models without existing connections Note that a model-name mismatch remains out of scope for now, as agreed in #1157. ### `get_connections()` In case of a failed List Connections call the `get_connections()` previously returned `{}` on any error. So every connection ID would read as "not found", essentially incorrectly indicating the cause. `get_connections()` now raises, and `post_publish_all()` logs the same ERROR line as before. With the flag set, the error header names the real cause once: "Could not list connections (...), so each connection ID below reads as not found". If no deployed model needed a binding, the error does not occur. Example error with the flag set: ```text Semantic model connection binding failed for 2 binding(s) while the 'enable_strict_semantic_model_binding' feature flag is set: - Semantic model 'ModelA': connection '00000000-0000-0000-0000-00000000dead' not found - Semantic model 'ModelB': connection '00000000-0000-0000-0000-00000000dead' not found ``` Example error with the flag set when the List Connections call fails: ```text Semantic model connection binding failed for 2 binding(s) while the 'enable_strict_semantic_model_binding' feature flag is set. Could not list connections (The executing identity is not authorized to call GET on 'https://api.fabric.microsoft.com/v1/connections'.), so each connection ID below reads as not found: - Semantic model 'ModelA': connection '11111111-1111-1111-1111-111111111111' not found - Semantic model 'ModelB': connection '11111111-1111-1111-1111-111111111111' not found ``` ### Remaining Questions & Extra notes on solution Below a few questions and clarifications. @shirasassoon looking forward to your reply. 1. One thing about the current behavior: a binding error can replace a `PublishError` of the same item type. So in the scenario where a semantic model fails to publish _and_ another binding fails, the caller gets only the binding error. We have deliberately left it this way to respect to comply with the current pattern for shortcut failures in `Lakehouse.post_publish_all()`, which works the same. If you prefer, we can immediately fix this or create a separate PR for it. 2. A stale connection ID on a model without existing connections counts as a failure, while a valid ID on the same model is a DEBUG skip (because binding needs an existing connection as a template) 3. For an excluded model, a connection ID that is not found still logs the existing WARNING. We have kept the log unchanged and not elevated this to failure status as well. ### Limitation(s) `get_connections()` reads only the first page of List Connections. Pagination is not handled yet. So this means that a valid connection ID on a later page reads as "not found". And considering a "not found" leads to an error with the flag, it fails the deployment. This bug exists in any case. For now, we kept it out of this PR to stay within the agreed scope. We would like to open a separate issue for it. It touches restricted FabricWorkspace however, so we'd require guidance on scope. ### Changes - `src/fabric_cicd/constants.py`: add `ENABLE_STRICT_SEMANTIC_MODEL_BINDING` to the `FeatureFlag` enum. - `src/fabric_cicd/_items/_semanticmodel.py`: return the failures from `bind_semanticmodel_to_connection()`, raise from `get_connections()`, and, with the flag set, raise in `post_publish_all()`. - `docs/how_to/optional_feature.md`: add the flag to the Publish behavior table. - `docs/how_to/parameterization.md`: describe the flag in the `semantic_model_binding` notes. - `tests/test_semantic_model_strict_binding.py`: new unit tests, with every failure path tested with the flag on and off. - `tests/test_bulk_publish.py`: one bulk-mode test. - `.changes/unreleased/added-20261017-225803.yaml`: changie fragment. ### Testing - `uv run pytest`: 1273 passed, 6 skipped, on Python 3.11 and on Python 3.9. - `uv run ruff check .`: all checks passed. - `uv run mkdocs build --clean`: the docs build, and the new link to `optional_feature.md#feature-flags` resolves. - A run against a real Fabric workspace. We've used the flag and a connection ID that does not exist. The notebook and the semantic model are published, and then the deployment fails with exit code 1: ```text [info] 15:20:50 - Enabled feature flag: enable_strict_semantic_model_binding 15:20:54 - Published Notebook 'canary_notebook' 15:21:12 - Published SemanticModel 'canary_model' [warn] 15:21:12 - Connection ID '00000000-0000-0000-0000-00000000dead' not found for semantic model 'canary_model' [error] 15:21:12 - Semantic model connection binding failed for 1 binding(s) while the 'enable_strict_semantic_model_binding' feature flag is set: - Semantic model 'canary_model': connection '00000000-0000-0000-0000-00000000dead' not found ``` ## Linked Issue (REQUIRED) Closes #1157 --------- Co-authored-by: shirasassoon <66449905+shirasassoon@users.noreply.github.com>