diff --git a/.github/prompts/swap-to-feature.prompt.md b/.github/prompts/swap-to-feature.prompt.md index 43a5b01..98f9f88 100644 --- a/.github/prompts/swap-to-feature.prompt.md +++ b/.github/prompts/swap-to-feature.prompt.md @@ -2,6 +2,15 @@ description: Swap the repo's IDs to your feature workspace and create the value set for the current branch. mode: agent --- -Run `python scripts/workspace_swap.py` from the repo root and report the output to me. The script will rewrite tracked Fabric files (semantic model, notebooks) so they point at your feature workspace instead of dev, create a feature value set, and update settings.json. It reads the feature workspace and lakehouse GUIDs from `.env` at the repo root — if `.env` is missing or empty, it will fall back to an interactive prompt. +The script `scripts/workspace_swap.py` rewrites tracked Fabric files (semantic model, notebooks) so they point at your feature workspace instead of dev, creates a feature value set, and updates settings.json. It reads `FEATURE_WORKSPACE_ID` and `FEATURE_LAKEHOUSE_ID` from `.env` at the repo root and asks for a `YES` confirmation before applying. When invoked through this prompt, that confirmation must happen here in chat — never in the terminal — to avoid live-prompt handling problems. -After the script finishes, summarize what changed and remind me to commit and push the changes, then sync the workspace from the Fabric UI. +Follow this exact sequence: + +1. Read `.env` at the repo root and read the dev workspace + lakehouse IDs from `data/fabric/Patterns_Variables.VariableLibrary/variables.json`. +2. Show me the planned swap in chat (current branch, dev → feature workspace ID, dev → feature lakehouse ID). +3. Use the chat UI to ask me a single question with two options: `YES` and `NO` (uppercase, exact). Do not proceed until I answer. +4. If I answer `YES`, run `echo "YES" | python scripts/workspace_swap.py`. The piped `YES` satisfies the script's confirmation gate non-interactively, so the terminal never blocks. Report the full output. +5. If I answer `NO`, do nothing further and confirm in chat that the swap was not applied. +6. After a successful run only, summarize what changed and remind me to commit and push the changes, then sync the workspace from the Fabric UI. + +Do not run the script directly without piping `YES` (it will block on the terminal prompt). Do not skip the chat confirmation — the in-script confirmation is bypassed by the pipe and the chat dialog is the gate. diff --git a/.github/workflows/deploy-prod.yml b/.github/workflows/deploy-prod.yml index ba7b082..a88b518 100644 --- a/.github/workflows/deploy-prod.yml +++ b/.github/workflows/deploy-prod.yml @@ -21,13 +21,13 @@ permissions: contents: read jobs: - deploy-supported: - name: Deploy supported items + deploy-fabric-cicd: + name: Deploy via fabric-cicd # Gated by the DEPLOY_METHOD repository variable. Runs when unset or set to # 'fabric-cicd'. Set DEPLOY_METHOD='bulk' to route deployments through # deploy-prod-bulk.yml instead. Any other value disables both workflows. if: vars.DEPLOY_METHOD == '' || vars.DEPLOY_METHOD == 'fabric-cicd' - uses: ./.github/workflows/reusable-deploy-supported.yml + uses: ./.github/workflows/reusable-deploy-fabric-cicd.yml with: environment: Prod item_type_in_scope: '["Lakehouse", "Ontology", "VariableLibrary", "Notebook", "SemanticModel", "Report", "DataAgent"]' diff --git a/.github/workflows/deploy-test.yml b/.github/workflows/deploy-test.yml index c9aecbd..65e5e3a 100644 --- a/.github/workflows/deploy-test.yml +++ b/.github/workflows/deploy-test.yml @@ -18,13 +18,13 @@ permissions: contents: read jobs: - deploy-supported: - name: Deploy supported items + deploy-fabric-cicd: + name: Deploy via fabric-cicd # Gated by the DEPLOY_METHOD repository variable. Runs when unset or set to # 'fabric-cicd'. Set DEPLOY_METHOD='bulk' to route deployments through # deploy-test-bulk.yml instead. Any other value disables both workflows. if: vars.DEPLOY_METHOD == '' || vars.DEPLOY_METHOD == 'fabric-cicd' - uses: ./.github/workflows/reusable-deploy-supported.yml + uses: ./.github/workflows/reusable-deploy-fabric-cicd.yml with: environment: Test item_type_in_scope: '["Lakehouse", "Ontology", "VariableLibrary", "Notebook", "SemanticModel", "Report", "DataAgent"]' diff --git a/.github/workflows/reusable-deploy-bulk.yml b/.github/workflows/reusable-deploy-bulk.yml index e541020..f873949 100644 --- a/.github/workflows/reusable-deploy-bulk.yml +++ b/.github/workflows/reusable-deploy-bulk.yml @@ -1,6 +1,6 @@ # Reusable workflow: Deploy supported Fabric items via the Bulk Import Item Definitions API (Preview). # -# Alternative deployment path to reusable-deploy-supported.yml. Uses the Fabric +# Alternative deployment path to reusable-deploy-fabric-cicd.yml. Uses the Fabric # REST API's bulk import endpoint instead of the fabric-cicd Python library. # Selected at orchestrator level via the DEPLOY_METHOD repository variable. # @@ -14,10 +14,19 @@ # - Every item type in the request payload must support service principals # (the bulk API requires SPN support for ALL items in the request, not just some) # -# Known gaps vs. reusable-deploy-supported.yml (fabric-cicd): -# - No parameter.yml find_replace / key_value_replace substitution -# - No orphan cleanup (Bulk Import API only supports Create/Update, not Delete) -# - No item_type_in_scope filter (deploys everything in repository_directory) +# Known gaps vs. reusable-deploy-fabric-cicd.yml (fabric-cicd): +# The Bulk Import API itself has no parameterization, no value-set +# activation, and no delete support. This repo bridges the first two in +# scripts/deploy_bulk.py + data/fabric/bulk-parameter.yml. The remaining +# gaps are intentionally not implemented: +# - No full parameter.yml feature coverage. bulk-parameter.yml supports +# find_replace + $items + $workspace + $environment placeholders only. +# fabric-cicd's key_value_replace, spark_pool, semantic_model_binding +# are not implemented here. +# - No orphan cleanup. The Bulk Import API only supports Create/Update, +# not Delete; deletes would need a separate per-item DELETE call loop. +# - No item_type_in_scope filter. Deploys everything under +# repository_directory. # # API references: # - Bulk import: https://learn.microsoft.com/en-us/rest/api/fabric/core/items/bulk-import-item-definitions(beta) diff --git a/.github/workflows/reusable-deploy-supported.yml b/.github/workflows/reusable-deploy-fabric-cicd.yml similarity index 93% rename from .github/workflows/reusable-deploy-supported.yml rename to .github/workflows/reusable-deploy-fabric-cicd.yml index a56eb40..db16527 100644 --- a/.github/workflows/reusable-deploy-supported.yml +++ b/.github/workflows/reusable-deploy-fabric-cicd.yml @@ -13,7 +13,7 @@ # - Fabric Admin must enable "Service principals can use Fabric APIs" # - parameter.yml in repository_directory for environment-specific replacements -name: "Reusable: Deploy Supported Items" +name: "Reusable: Deploy via fabric-cicd" on: workflow_call: @@ -38,7 +38,7 @@ permissions: jobs: deploy: - name: Deploy supported items (${{ inputs.environment }}) + name: Deploy via fabric-cicd (${{ inputs.environment }}) runs-on: ubuntu-latest timeout-minutes: 30 environment: ${{ inputs.environment }} diff --git a/README.md b/README.md index 3212868..ebd8b1b 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,8 @@ Branch protection (PR required, source-branch restrictions, status checks) is en | Document | Description | |---|---| | [CI/CD Release Options](fabric-cicd-release-options.md) | Evaluates all CI/CD release options for Fabric (Deployment Pipelines, Git-based, Build-based, Hybrid) and recommends the Hybrid approach. Includes a [comparison of fabric-cicd vs the new Bulk Import / Export APIs](fabric-cicd-release-options.md#tooling-within-option-3-fabric-cicd-vs-bulk-apis) (Preview) within Option 3. **Start here** if you're deciding on a strategy. | -| [Hybrid CI/CD Implementation Guide](fabric-hybrid-cicd-guide.md) | Deep dive into the implementation: workflow structure, configuration strategy, prerequisites, setup steps, and gotchas. | +| [Hybrid CI/CD Implementation Guide](fabric-hybrid-cicd-guide.md) | Deep dive into the recommended fabric-cicd implementation: workflow structure, configuration strategy, prerequisites, setup steps, and gotchas. | +| [Bulk CI/CD Implementation Guide](fabric-bulk-cicd-guide.md) | Implementation guide for the alternative Bulk Import API (Preview) deploy path. Covers the gap-bridging workarounds (substitution, value-set activation), the two-deploy decision, extension patterns, and limitations not bridged. | | [Development Process](fabric-development-process.md) | How developers work day-to-day: branch-out workflow, the workspace swap script, and PR readiness check. | | [CI/CD Governance Considerations](fabric-cicd-governance-considerations.md) | Considerations on identities, RBAC, branch protection, and approval gates for the CI/CD pipeline. Includes pointers to adjacent controls owned outside the pipeline (security/compliance topics). | diff --git a/fabric-bulk-cicd-guide.md b/fabric-bulk-cicd-guide.md new file mode 100644 index 0000000..9a8675c --- /dev/null +++ b/fabric-bulk-cicd-guide.md @@ -0,0 +1,563 @@ +# Bulk CI/CD Implementation Guide + +This repository implements a parallel deployment path for Microsoft Fabric using the **[Bulk Import Item Definitions API](https://learn.microsoft.com/en-us/rest/api/fabric/core/items/bulk-import-item-definitions(beta))** (Preview), as an alternative to the [fabric-cicd path](fabric-hybrid-cicd-guide.md). It demonstrates how to deploy the same Fabric workspace items (Notebooks, Lakehouses, Variable Libraries, Semantic Models, Reports, Ontologies, Data Agents) across environments using GitHub Actions and the Fabric REST API directly. + +For the strategic comparison between fabric-cicd and the Bulk APIs (and the recommendation), see [fabric-cicd-release-options.md](fabric-cicd-release-options.md#tooling-within-option-3-fabric-cicd-vs-bulk-apis). + +> **Important framing.** The Bulk Import API itself has known gaps (no parameterization, no value-set activation, no delete). This repo implements caller-side workarounds for the first two so the demo works end-to-end — they are not platform fixes. If you choose the bulk path in your own project you take on the same caller-side work. fabric-cicd remains the recommended production path; this guide exists so the bulk pattern is documented as a worked example, not an endorsement. + +--- + +## Table of Contents + +- [Architecture Overview](#architecture-overview) +- [Repository Structure](#repository-structure) +- [Deployment Flow](#deployment-flow) +- [GitHub Actions Workflows](#github-actions-workflows) +- [Configuration Strategy](#configuration-strategy) +- [The Two-Deploy Decision](#the-two-deploy-decision) +- [Prerequisites & Setup](#prerequisites--setup) +- [Initial Deployment to a Clean Workspace](#initial-deployment-to-a-clean-workspace) +- [Gotchas & Key Decisions](#gotchas--key-decisions) +- [Extending the Bulk Implementation](#extending-the-bulk-implementation) +- [Limitations Not Bridged](#limitations-not-bridged) +- [References](#references) + +--- + +## Architecture Overview + +``` +Git repo (dev branch) + │ + │ PR merge → test branch + ▼ +┌─────────────────────────────────────────────────────┐ +│ deploy-test-bulk.yml (orchestrator) │ +│ │ +│ deploy-bulk │ +│ └─ reusable-deploy-bulk.yml │ +│ └─ scripts/deploy_bulk.py │ +│ (Phase 1: POST dependencies │ +│ — Lakehouse + Ontology) │ +│ (Phase 2: substitute IDs, │ +│ POST remaining items) │ +│ (Post-deploy: PATCH VariableLibrary │ +│ active value set) │ +└─────────────────────────────────────────────────────┘ + │ workflow_run (on success) + ▼ +┌─────────────────────────────────────────────────────┐ +│ etl-test.yml │ +│ └─ reusable-fabric-etl.yml │ +│ └─ Fabric REST API: run notebook by name │ +└─────────────────────────────────────────────────────┘ +``` + +The same pattern applies to Prod (`deploy-prod-bulk.yml` → `etl-prod.yml`), triggered on push to `main`. + +The shape mirrors the [fabric-cicd path](fabric-hybrid-cicd-guide.md#architecture-overview) deliberately. Both paths split the deploy into two phases for the same reason — the first phase creates items whose IDs the second phase needs to reference. The differences are mechanical: + +| Concept | fabric-cicd | bulk | +|---|---|---| +| Calls per phase | One library call (`publish_all_items()`) per phase, which makes many per-item REST calls internally | One bulk POST per phase carrying the full batch | +| Substitution | fabric-cicd library applies `parameter.yml` rules transparently | `scripts/deploy_bulk.py` reads `bulk-parameter.yml` and rewrites payloads between phases | +| Value-set activation | Library handles automatically when `environment` is passed | Caller makes a separate `PATCH /variableLibraries/{id}` call | +| Orphan cleanup | `unpublish_all_orphan_items()` built in | Not implemented | + +> An alternative fabric-cicd deploy path exists alongside this bulk path; both are gated by the `DEPLOY_METHOD` repo variable. fabric-cicd is the recommended path — see [fabric-cicd vs Bulk APIs](fabric-cicd-release-options.md#tooling-within-option-3-fabric-cicd-vs-bulk-apis) for the comparison and [fabric-hybrid-cicd-guide.md](fabric-hybrid-cicd-guide.md) for its implementation guide. + +### Branches & Workspaces + +| Branch | Workspace | Deployment Method | +|---|---|---| +| `dev` | Dev (microsoft-fabric-sdlc-patterns-dev) | Git-connected via Fabric Git integration | +| `test` | Test (microsoft-fabric-sdlc-patterns-test) | Bulk Import API via GitHub Actions | +| `main` | Prod (microsoft-fabric-sdlc-patterns-prod) | Bulk Import API via GitHub Actions | + +- **Dev** workspace is the only Git-connected workspace. Developers branch out from Dev for isolated feature work. +- **Test** and **Prod** workspaces are NOT Git-connected. With the bulk path active, they receive deployments through `scripts/deploy_bulk.py`. + +--- + +## Repository Structure + +``` +microsoft-fabric-sdlc-patterns/ +├── .github/ +│ └── workflows/ +│ ├── deploy-test-bulk.yml # Orchestrator: push to test → bulk deploy +│ ├── deploy-prod-bulk.yml # Orchestrator: push to main → bulk deploy +│ ├── etl-test.yml # Triggers after either deploy-test* succeeds +│ ├── etl-prod.yml # Triggers after either deploy-prod* succeeds +│ ├── reusable-deploy-bulk.yml # Template: Bulk Import API deployment +│ ├── reusable-fabric-etl.yml # Template: run Notebook via Fabric REST API +│ ├── check-pr-ready.yml # PR check: blocks feature IDs from merging to dev +│ ├── run-tests.yml # PR check: runs pytest when scripts/tests change +│ └── enforce-promotion-path.yml # PR check: enforces dev→test→main source-branch promotion +├── data/ +│ └── fabric/ # Fabric item definitions (repository_directory) +│ ├── bulk-parameter.yml # Bulk path's parameterization (independent format) +│ ├── parameter.yml # fabric-cicd parameterization (ignored by bulk) +│ ├── PatternsLakehouse.Lakehouse/ +│ ├── Patterns_Ontology.Ontology/ +│ ├── Patterns_Variables.VariableLibrary/ +│ ├── Import_Patterns_Data.Notebook/ +│ ├── Patterns_Patients_Data.Notebook/ +│ ├── Patterns_Demo.Notebook/ +│ ├── Patterns_Semantic_Model.SemanticModel/ +│ ├── Patterns_Report.Report/ +│ └── Patterns_Data_Agent.DataAgent/ +├── scripts/ +│ ├── deploy_bulk.py # Bulk Import API deploy (invoked by reusable-deploy-bulk.yml) +│ ├── deploy_fabric_cicd.py # fabric-cicd deploy (alternative path) +│ ├── run_fabric_etl.py # Run a Fabric Notebook job (invoked by reusable-fabric-etl.yml) +│ └── workspace_swap.py # Bootstrap/reset feature branch workspace bindings +├── tests/ +│ └── test_deploy_bulk.py # Unit tests for the bulk script +└── ... (other docs, see README) +``` + +The fabric-cicd workflows (`deploy-test.yml`, `deploy-prod.yml`, `reusable-deploy-fabric-cicd.yml`) coexist with the bulk workflows in the same `.github/workflows/` directory. The `DEPLOY_METHOD` repository variable selects which one runs. + +--- + +## Deployment Flow + +### What Triggers What + +| Event | Workflow Triggered | What It Does | +|---|---|---| +| Push to `test` branch (changes in `data/fabric/**`) when `DEPLOY_METHOD=bulk` | `deploy-test-bulk.yml` | Deploys all items to the Test workspace via the Bulk Import API | +| `deploy-test-bulk.yml` completes successfully | `etl-test.yml` | Runs the `Import_Patterns_Data` notebook in the Test workspace | +| Push to `main` branch (changes in `data/fabric/**`) when `DEPLOY_METHOD=bulk` | `deploy-prod-bulk.yml` | Deploys all items to the Prod workspace via the Bulk Import API | +| `deploy-prod-bulk.yml` completes successfully | `etl-prod.yml` | Runs the `Import_Patterns_Data` notebook in the Prod workspace | + +The `DEPLOY_METHOD` repository variable (Settings → Secrets and variables → Actions → Variables) controls which deploy workflow runs: + +| `DEPLOY_METHOD` value | Behavior | +|---|---| +| `fabric-cicd` *(or unset)* | fabric-cicd workflows run; bulk workflows skip | +| `bulk` | Bulk workflows run; fabric-cicd workflows skip | +| any other value | Both deploy workflows skip (safe default) | + +### The Bulk Deploy Job + +Each deploy workflow calls `reusable-deploy-bulk.yml`, which invokes `scripts/deploy_bulk.py`. The script: + +1. Acquires an Entra ID bearer token using the SPN's client credentials. +2. Walks `data/fabric/` and builds a `definitionParts[]` array — one entry per file, with the path and base64-encoded content. +3. Loads `bulk-parameter.yml` to get the substitution rules and value-set activation config. +4. Decides single-deploy vs. two-deploy based on whether any rule references `$items...$id`. +5. POSTs to `https://api.fabric.microsoft.com/v1/workspaces/{ws}/items/bulkImportDefinitions?beta=true` (one or two times depending on the decision). +6. PATCHes the deployed VariableLibrary to set the active value set. + +The Bulk Import API can return `200 OK` with the result body inline, or `202 Accepted` with a Long-Running Operation (LRO). The script handles both transparently — see [Gotchas](#lro-polling). + +The ETL workflow triggers automatically after the deploy workflow completes successfully. If the deploy fails, ETL does not run. + +--- + +## GitHub Actions Workflows + +### Reusable Templates (called via `workflow_call`) + +| Template | Purpose | +|---|---| +| `reusable-deploy-bulk.yml` | Acquires a token (via the SPN secrets), checks out the repo, installs `requests` + `PyYAML`, and invokes `scripts/deploy_bulk.py` with the workspace ID, repository directory, and target environment as env vars. | +| `reusable-fabric-etl.yml` | Resolves a Fabric item by **name** (not ID) via the List Items API, then starts a job (RunNotebook) and polls until completion. Shared with the fabric-cicd path — same template, no changes needed. | + +### Why Reusable Workflows (Not Composite Actions) + +Reusable workflows support the `environment:` keyword at the job level, which enables: + +- **GitHub Environment protection rules** (required reviewers, branch restrictions on Prod) +- **Environment-scoped secrets** (each environment has its own `FABRIC_WORKSPACE_ID`) +- `secrets: inherit` forwards all environment secrets without enumeration + +### Manual Re-runs via `workflow_dispatch` + +`etl-test.yml` and `etl-prod.yml` both support `workflow_dispatch` so an operator can manually re-trigger the ETL job from the Actions UI without a new deploy. This was added to work around a `workflow_run` quirk: re-running a `workflow_run`-triggered run uses the workflow file frozen at the original trigger time, not the current default branch. For ad-hoc recovery from transient failures or workflow-file fixes that just landed on `main`, the manual dispatch is the path of least friction. + +--- + +## Configuration Strategy + +Three complementary mechanisms handle environment-specific configuration in the bulk path. The first two mirror the [fabric-cicd path's strategy](fabric-hybrid-cicd-guide.md#configuration-strategy); the third is bulk-specific because the API itself doesn't activate value sets. + +### 1. Variable Libraries (Runtime) + +Notebooks call `notebookutils.variableLibrary.getLibrary("Patterns_Variables")` at execution time to resolve workspace IDs, lakehouse names, and other values. The Variable Library has **value sets** per environment: + +| Variable | Default (Dev) | Test | Prod | +|---|---|---|---| +| `target_workspace_id` | Dev workspace ID | Test workspace ID | Prod workspace ID | +| `target_workspace_name` | `microsoft-fabric-sdlc-patterns-dev` | `microsoft-fabric-sdlc-patterns-test` | `microsoft-fabric-sdlc-patterns-prod` | +| `target_lakehouse_name` | `PatternsLakehouse` | *(default)* | *(default)* | +| `target_lakehouse_id` | Dev lakehouse ID | Dev lakehouse ID* | Dev lakehouse ID* | + +\* The `target_lakehouse_id` uses the Dev GUID as a placeholder in the value set files. At deploy time, `bulk-parameter.yml` rewrites it with the actual lakehouse ID in the target workspace (see below). + +### 2. `bulk-parameter.yml` (Deploy-time) + +This is the bulk path's equivalent of fabric-cicd's `parameter.yml`. The format is intentionally distinct — bulk implements a narrow subset of fabric-cicd's DSL, only what this repo's deploys need. + +The shipped file at `data/fabric/bulk-parameter.yml` looks like this: + +```yaml +substitutions: + - find: "c185283c-9dd9-4e40-a17c-aa6303e3a2e9" # dev lakehouse ID + replace_with: "$items.Lakehouse.PatternsLakehouse.$id" + item_types: [VariableLibrary, SemanticModel, Notebook] + + - find: "d7270f11-feba-4990-baa6-d45e47f23737" # dev workspace ID + replace_with: "$workspace.$id" + item_types: [SemanticModel, Notebook] + +variable_library: + active_value_set: "$environment" +``` + +**Schema:** + +| Top-level key | Purpose | +|---|---| +| `substitutions` | List of find/replace rules. Each rule has `find` (the literal string), `replace_with` (the replacement, with optional placeholders), and `item_types` (list of item types this rule applies to). | +| `variable_library` | Per-VariableLibrary settings. Currently just `active_value_set`. | + +**Placeholder reference:** + +| Placeholder | Resolved to | +|---|---| +| `$workspace.$id` | The target workspace ID (from the `FABRIC_WORKSPACE_ID` env var) | +| `$items...$id` | The deployed item ID for `/`. Resolved from Phase 1's bulk POST response. | +| `$environment` | The value of the `ENVIRONMENT` env var passed to the deploy step (`Test` or `Prod`). | + +**`item_types` filter:** A rule only fires for files whose item type is in the rule's `item_types` list. The script extracts the item type from the path convention `./`. Files outside the listed types pass through unchanged. + +**Why a separate file from `parameter.yml`:** The two formats coexist deliberately. fabric-cicd uses `parameter.yml`; bulk uses `bulk-parameter.yml`. Keeping them separate means bulk doesn't have to silently ignore (or break on) fabric-cicd-only features (`key_value_replace`, `spark_pool`, `semantic_model_binding`). Both files live at the root of `data/fabric/` and both are excluded from the bulk request payload by `scripts/deploy_bulk.py`. + +### 3. VariableLibrary Value-Set Activation (Post-deploy) + +The Bulk Import API uploads the VariableLibrary's value-set files but does NOT set which one is active in the deployed workspace. fabric-cicd makes that selection automatically via its `environment` parameter; with the bulk API, the caller must do it. + +`scripts/deploy_bulk.py` makes the call after the bulk POSTs complete: + +``` +PATCH /v1/workspaces/{workspace_id}/variableLibraries/{library_id} +Content-Type: application/json + +{ + "properties": { + "activeValueSetName": "Test" + } +} +``` + +The activation is gated on `bulk-parameter.yml`'s `variable_library.active_value_set` being non-null. The placeholder `$environment` resolves to the target environment name (`Test` or `Prod`). If you set the field to a literal string instead, that exact value is used. + +Reference: [Update Variable Library](https://learn.microsoft.com/en-us/rest/api/fabric/variablelibrary/items/update-variable-library). + +--- + +## The Two-Deploy Decision + +The bulk script auto-decides between a single-deploy and two-deploy flow based on what's in `bulk-parameter.yml`. The decision is deterministic: + +```python +needs_two_deploy = any("$items." in r.replace_with for r in config.substitutions) +``` + +If any substitution rule references `$items...$id`, the deploy must split in two — the script needs the item IDs from Phase 1's response before it can substitute them into Phase 2's payloads. + +### Single-deploy flow (no `$items.*` references, or no config at all) + +``` +1. apply_substitutions(parts, rules, workspace_id, item_id_map={}) +2. POST all items at once +3. PATCH VariableLibrary if value-set activation is configured +``` + +`$workspace.$id`-only rules are applied in this flow because the workspace ID is known up front from the env var. + +### Two-deploy flow (rules reference `$items.*`) + +``` +1. partition_dependencies(parts) → (deps, remaining) +2. POST deps ┐ +3. extract_item_ids(deps_response) │ Phase 1 + → item_id_map = {(Type, Name): item_id} ┘ +4. apply_substitutions(remaining, rules, ┐ + workspace_id, │ + item_id_map) │ Phase 2 +5. POST remaining ┘ +6. PATCH VariableLibrary if value-set activation is configured +``` + +`DEPENDENCY_TYPES` in `scripts/deploy_bulk.py` defines what counts as a dependency. The list is intentionally narrow — only types actually referenced by `$items..*` in `bulk-parameter.yml` belong here. For this repo, that's `("Lakehouse", "Ontology")`. + +This mirrors the fabric-cicd path's two-phase deploy — see the [hybrid guide's chicken-and-egg gotcha](fabric-hybrid-cicd-guide.md#chicken-and-egg-lakehouse-id) for the same problem framed for fabric-cicd. + +### When the script fails fast + +The script raises a clear error in two situations: + +- **Substitution rules reference `$items...$id`, but no items of dependency types are found in the repo.** The deploy can't satisfy the placeholder. +- **A rule references an item the script can't find by `(Type, Name)`.** The placeholder won't resolve. Common cause: typo in `bulk-parameter.yml`. + +Both cases stop the deploy before any items are POSTed. + +--- + +## Prerequisites & Setup + +### 1. Fabric Capacity + +A Fabric or Power BI Premium capacity is required for all workspaces. + +### 2. Fabric Workspaces + +Three workspaces are needed: + +- **microsoft-fabric-sdlc-patterns-dev** — connected to the `dev` branch via Fabric Git integration +- **microsoft-fabric-sdlc-patterns-test** — not Git-connected, receives deployments via the Bulk Import API +- **microsoft-fabric-sdlc-patterns-prod** — not Git-connected, receives deployments via the Bulk Import API + +### 3. Service Principal + +Create a Service Principal for CI/CD automation: + +```bash +az ad sp create-for-rbac --name "SPN-Microsoft-Fabric-SDLC-Patterns" \ + --query "{tenantId:tenant, clientId:appId, clientSecret:password}" -o json +``` + +- Add the SPN as **Contributor** on both Test and Prod workspaces (Workspace → Manage access → Add people or groups). +- Contributor is the minimum required role per the [Fabric Create Item API](https://learn.microsoft.com/en-us/rest/api/fabric/core/items/create-item) documentation. +- **Critical for bulk:** every item type in your repo must support service principals. The Bulk Import API enforces SPN coverage at the request level — a single unsupported item type fails the whole call. See the [comparison table in release-options](fabric-cicd-release-options.md#tooling-within-option-3-fabric-cicd-vs-bulk-apis) for context. + +> **Important:** A Fabric Admin must enable service principal access to Fabric APIs in the Fabric Admin portal under Developer settings, scoped to a security group containing only your CI/CD SPs. See [developer tenant settings](https://learn.microsoft.com/en-us/fabric/admin/service-admin-portal-developer) and the [Governance Considerations](fabric-cicd-governance-considerations.md) for details. + +### 4. GitHub Environments + +Create two GitHub Environments in the repository settings (Settings → Environments): + +| Environment | Protection Rules | +|---|---| +| `Test` | None (deploy flows automatically on merge) | +| `Prod` | Required reviewers, deployment branch restriction to `main` only | + +> **Note:** The environment name is also used as the value substituted for `$environment` in `bulk-parameter.yml`'s `active_value_set` placeholder. So the GitHub Environment names must match the Variable Library value-set names (`Test`, `Prod`) exactly. + +### 5. GitHub Environment Secrets + +Add these secrets to **both** `Test` and `Prod` environments: + +| Secret | Description | +|---|---| +| `AZURE_TENANT_ID` | Entra ID tenant ID | +| `AZURE_CLIENT_ID` | Service Principal client/app ID | +| `AZURE_CLIENT_SECRET` | Service Principal client secret | +| `FABRIC_WORKSPACE_ID` | Target workspace ID (different per environment) | + +The first three secrets are identical across environments (single SPN). `FABRIC_WORKSPACE_ID` differs: + +- Test: the Test workspace ID +- Prod: the Prod workspace ID + +### 6. Set the `DEPLOY_METHOD` Repository Variable + +To activate the bulk path, set `DEPLOY_METHOD=bulk` in Settings → Secrets and variables → Actions → Variables. Without this, the bulk workflows skip and fabric-cicd runs instead. + +--- + +## Initial Deployment to a Clean Workspace + +When deploying to a workspace for the first time, follow these steps in order. Subsequent deployments are fully automated — only the first deployment requires manual intervention. + +### Step 1: Trigger the Deployment + +Push to the target branch (`test` or `main`). The bulk deploy workflow triggers automatically and executes: + +- **Phase 1 (if needed):** POST Lakehouse + Ontology +- **Phase 2:** Substitute IDs from Phase 1's response into the remaining items, then POST them +- **Post-deploy:** PATCH the VariableLibrary to set the active value set for the environment + +### Step 2: ETL Populates the Lakehouse + +The ETL workflow (`etl-test.yml` or `etl-prod.yml`) triggers automatically after a successful deployment. It runs the `Import_Patterns_Data` notebook, which creates and populates the Delta tables (`doctors`, `patients`, `appointments`) in the Lakehouse. + +### Step 3: Configure Graph Model Data Source (Manual) + +The Ontology is deployed as a definition only — its Graph Model does not have a data source binding until you configure it manually. + +1. Open the **Ontology** item in the Fabric UI and navigate to the **Graph Model**. +2. Select **Get data** to bind the Graph Model to the lakehouse tables. + +### Step 4: Activate the Ontology (Manual Workaround) + +After configuring the data source, the Ontology may remain stuck on *"Setting up your ontology — We are preparing the ontology overview for the first time."* This is a known Fabric platform behavior on initial deployment. + +**Workaround:** Select any Entity Type in the Ontology, rename it to something temporary, then rename it back to its original name. This triggers Fabric to finish initializing the Ontology overview. + +### Step 5: Verify End-to-End + +Confirm all items are functional in the target workspace: + +- **Lakehouse** — tables populated with data +- **Ontology** — overview loads, entity types and relationships visible +- **Variable Library** — active value set matches the target environment (`Test` or `Prod`) +- **Semantic Model** — connected to the lakehouse (may require manual connection config on first deploy) +- **Report** — renders with data from the Semantic Model +- **Data Agent** — references the Ontology and responds to queries + +> **Note:** Steps 3–4 (Ontology activation) are platform behaviors, not bulk-specific. The fabric-cicd path requires the same manual steps on first deploy. + +--- + +## Gotchas & Key Decisions + +### Two-Deploy Chicken-and-Egg + +The Variable Library, Semantic Model, and notebooks reference the lakehouse ID for each environment, but the lakehouse doesn't exist in Test/Prod until the first deployment creates it. With the bulk API, there's no library-side resolution of these references — the script must POST the dependency items first, read their IDs from the response, then substitute into the remaining items before the second POST. + +**Solution:** [The two-deploy decision](#the-two-deploy-decision). Phase 1 POSTs the dependencies (`DEPENDENCY_TYPES`), the script reads their IDs from the response, then Phase 2 substitutes and POSTs the rest. + +### Per-Request Service Principal Coverage + +The Bulk Import API requires that **every** item type in the request payload supports service principals. If even one type doesn't, the entire request fails with an authorization error — there's no graceful degradation. + +fabric-cicd, by contrast, fails per item — an unsupported item type fails only that item, and the rest of the deploy continues. + +**Solution:** Verify SPN coverage for every item type in your repo before adopting the bulk path. As of Phase 2, all 7 types in this repo (Lakehouse, Ontology, VariableLibrary, Notebook, SemanticModel, Report, DataAgent) support SPNs and the deploy works end-to-end. If a future Fabric item type doesn't support SPNs, the bulk path won't be usable until coverage is added. + +### LRO Polling + +The Bulk Import API can return either: + +- **`200 OK`** with the full result body (`importItemDefinitionsDetails[]`) inline — synchronous case +- **`202 Accepted`** with an `x-ms-operation-id` header and a `Retry-After` header — asynchronous, caller polls a Long-Running Operation + +`scripts/deploy_bulk.py` handles both transparently. For the LRO case, it polls `GET /v1/operations/{id}` until the operation reaches `Succeeded`, `Failed`, or `Undefined`, then fetches `GET /v1/operations/{id}/result` for the same `importItemDefinitionsDetails[]` shape the sync case returns inline. + +### Retry-After Clamping + +The script clamps the `Retry-After` header to `[5s, 600s]` and falls back to 30s on missing or unparseable values. Without the upper bound, a pathological `Retry-After` could cause a single sleep longer than the global polling timeout (20 minutes), making the timeout meaningless. Without the input validation, a non-integer value would crash the script with an unhandled `ValueError`. + +### Bearer Token Is Not Echoed to Logs + +An earlier version of the script emitted `::add-mask::` to register a workflow log mask. That line itself emitted the token to stdout in cleartext, defeating the mask's purpose (the runner only redacts subsequent output). The current script never prints the token. The SPN secret is masked by GitHub automatically because it comes from `secrets.AZURE_CLIENT_SECRET`. + +### `bulk-parameter.yml` Is Excluded from the Request Payload + +The script lists `bulk-parameter.yml`, `parameter.yml`, and `.gitkeep` in `EXCLUDED_FILES` so they're never sent to Fabric as part of the bulk request. The structural rule "files at the root of `repository_directory` are not item definitions" already excludes them, but the explicit list documents intent. + +### `workflow_run` Quirk: Re-runs Use the Frozen Workflow File + +Re-running a `workflow_run`-triggered ETL run uses the workflow file frozen at the time the trigger fired, not the current default branch. If a workflow-file fix lands on `main` after the trigger fires, neither the auto-retry nor `gh run rerun` picks it up. To recover from a transient failure or a workflow-file fix, use the `workflow_dispatch` manual trigger added to `etl-test.yml` and `etl-prod.yml`. + +### `?beta=true` Required Today + +The bulk endpoint URL is currently `POST /v1/workspaces/{ws}/items/bulkImportDefinitions?beta=true`. When the API graduates from Preview, drop the query parameter. The TODO is noted in the script's module docstring and in `reusable-deploy-bulk.yml`. + +### Microsoft Tutorial's URL Is Wrong + +The Microsoft Learn [Bulk Import tutorial](https://learn.microsoft.com/en-us/fabric/cicd/tutorial-bulkapi-cicd) uses `/importItemDefinitions` (singular) — that endpoint produces `404`. The correct path is `/items/bulkImportDefinitions` per the [API reference page](https://learn.microsoft.com/en-us/rest/api/fabric/core/items/bulk-import-item-definitions(beta)). Verified against a live workspace. + +--- + +## Extending the Bulk Implementation + +The script is structured so common extension cases are localized. Here's the shape of each. + +### Adding a new substitution rule + +Edit `data/fabric/bulk-parameter.yml`: + +```yaml +substitutions: + # ... existing rules ... + - find: "" + replace_with: "" + item_types: [Notebook, SemanticModel] +``` + +No code change required. The new rule is picked up automatically on the next deploy. + +If the new rule references `$items...$id` for a new item type, also extend `DEPENDENCY_TYPES` in `scripts/deploy_bulk.py` so that type deploys in Phase 1. + +### Adding a new placeholder type + +To add e.g. `$secrets.` resolving to a CI secret, modify `scripts/deploy_bulk.py`: + +1. Add a constant `_SECRETS_PLACEHOLDER = re.compile(r"\$secrets\.([^.\s]+)")`. +2. In `resolve_dynamic_value()`, add a `_SECRETS_PLACEHOLDER.sub(...)` call. +3. Add a corresponding env var (e.g. `BULK_SECRETS_JSON`) and read it in `main()`. +4. Add tests for the new resolver. + +The pattern follows what `$workspace.$id` and `$items...$id` already do. + +### Adding a new dependency type + +If a new item type becomes a target of `$items..*` substitutions (e.g. a new Warehouse that other items depend on): + +```python +DEPENDENCY_TYPES = ("Lakehouse", "Ontology", "Warehouse") +``` + +That's the only code change. Phase 1 will start including the new type; Phase 2 substitutions will resolve its IDs. + +### Adding a new post-deploy hook + +The VariableLibrary value-set activation is the existing example. To add another (e.g. creating a Lakehouse Shortcut after the deploy): + +1. Add a new helper, e.g. `create_shortcut(workspace_id, lakehouse_id, shortcut_config, headers)` that wraps the appropriate Fabric REST API call. +2. Add a corresponding config block to `BulkConfig` (e.g. `shortcuts: tuple[ShortcutConfig, ...]`). +3. Extend `load_bulk_config()` to parse the new block from `bulk-parameter.yml`. +4. Wire the hook in `main()` after the deploy, alongside the existing value-set activation. +5. Add unit tests for the helper and the config parsing. + +The pattern is intentionally repeatable — every post-deploy step is "config block in YAML → dataclass field → helper function → call site in `main()`." + +### Adding a new bulk-config block + +The shipped config has `substitutions` and `variable_library`. To add a third block (e.g. `shortcuts` per above): + +1. Add a new `@dataclass(frozen=True)` for the block's contents (e.g. `ShortcutConfig`). +2. Add it as a field on `BulkConfig` with a sensible default. +3. Extend `load_bulk_config()` to parse the new block, with explicit error messages for malformed input. +4. Add tests covering the happy path, missing-block fallback, and each documented error case. + +The existing `VariableLibraryConfig` is a working example. + +--- + +## Limitations Not Bridged + +This repo's bulk path implements substitution and value-set activation but does NOT bridge the following. If you choose the bulk path in your own project, you'll need to implement these or accept their absence. + +| Limitation | What's missing | Workaround if you need it | +|---|---|---| +| **Orphan cleanup** | Bulk Import API only supports Create/Update — no Delete. Items removed from the repo stay in the workspace. | Maintain a separate per-item `DELETE` loop after the bulk POST. | +| **`key_value_replace`** | fabric-cicd's JSONPath-based key replacement (e.g., connection IDs in pipeline JSON). | Extend `apply_substitutions()` to support a JSONPath-style replacement step in addition to literal find/replace. | +| **`spark_pool` substitution** | fabric-cicd's per-environment Spark pool swapping. | Add a config block + post-deploy API call (similar to value-set activation). | +| **`semantic_model_binding`** | fabric-cicd's auto-binding of semantic models to data source connections. | Add a post-deploy API call against the Semantic Model. | +| **Per-item SPN graceful degradation** | Bulk fails the whole request if any item type is unsupported by SPNs. | Pre-flight check: list item types in the request, validate against a known-supported set. | +| **Multi-VariableLibrary handling** | `find_variable_library_id()` raises if multiple VariableLibraries are present (the activation step targets one). | Refactor the activation flow to take a per-library config and iterate. | +| **`item_type_in_scope` filter** | The script deploys everything in `repository_directory`. No way to scope a deploy to a subset. | Add an env var read in `main()` and filter parts before the POST. | + +These are deliberate non-goals for this demo repo. They can be added incrementally using the [extension patterns](#extending-the-bulk-implementation) above. + +--- + +## References + +- [fabric-cicd-release-options.md](fabric-cicd-release-options.md) — Strategy doc with the fabric-cicd vs Bulk APIs comparison +- [fabric-hybrid-cicd-guide.md](fabric-hybrid-cicd-guide.md) — Implementation guide for the fabric-cicd path +- [fabric-cicd-governance-considerations.md](fabric-cicd-governance-considerations.md) — Identity, RBAC, branch protection, approval gates +- [Fabric Bulk Import Item Definitions API (Preview)](https://learn.microsoft.com/en-us/rest/api/fabric/core/items/bulk-import-item-definitions(beta)) — Endpoint reference +- [Fabric Long-Running Operations](https://learn.microsoft.com/en-us/rest/api/fabric/articles/long-running-operation) — `?async=true` semantics, polling pattern +- [Fabric Update Variable Library](https://learn.microsoft.com/en-us/rest/api/fabric/variablelibrary/items/update-variable-library) — PATCH endpoint used by value-set activation +- [Microsoft Bulk Import tutorial](https://learn.microsoft.com/en-us/fabric/cicd/tutorial-bulkapi-cicd) — Microsoft's walkthrough (note the URL discrepancy called out under [Gotchas](#microsofts-tutorial-url-is-wrong)) +- [Fabric Create Item API — Permissions](https://learn.microsoft.com/en-us/rest/api/fabric/core/items/create-item) — Contributor role requirement +- [GitHub Reusable Workflows](https://docs.github.com/en/actions/sharing-automations/reusing-workflows) — `workflow_call`, inputs, secrets diff --git a/fabric-cicd-release-options.md b/fabric-cicd-release-options.md index 2333979..66d7fdd 100644 --- a/fabric-cicd-release-options.md +++ b/fabric-cicd-release-options.md @@ -260,13 +260,20 @@ Both implementations sit inside Option 3 — branch per stage, build environment | Dimension | fabric-cicd | Bulk Import / Export APIs | |---|---|---| | **Maturity** | GA | Preview (`?beta=true` query parameter required) | -| **Environment-specific config** | `parameter.yml` (declarative `find_replace`, `key_value_replace`, `$items` resolution) | None — caller must preprocess files or rely entirely on Variable Libraries + logical IDs | +| **Environment-specific config** | `parameter.yml` (declarative `find_replace`, `key_value_replace`, `$items` resolution) | None at the API level — caller must preprocess files or rely entirely on Variable Libraries + logical IDs. *This repo demonstrates one way to bridge the gap in caller code (see note below the table).* | | **Orphan cleanup** | `unpublish_all_orphan_items()` built in | None — API only supports Create / Update; deletes are caller's responsibility | | **Dependency ordering** | Caller phases manually (e.g., Lakehouse + Ontology first, then everything else) | Service resolves automatically in a single call | | **Long-running operations** | Hidden by the library | When the call returns `202 Accepted`, the caller polls `/operations/{id}` and then `/operations/{id}/result` explicitly. Sync `200 OK` returns the result body directly with no polling needed. | | **API call shape** | Many per-item REST calls | One POST for the entire workspace payload | | **Service principal coverage** | Per item — an unsupported item type fails only that item | Per request — service principals are supported only when *every* item in the payload supports service principals | +> **Note on the gaps in this repo.** The Bulk Import API gaps above are properties of the API itself — Microsoft has not added these capabilities. This repo implements two of them in caller code so the demo works end-to-end: +> +> - **Substitution:** `data/fabric/bulk-parameter.yml` + `scripts/deploy_bulk.py` apply find/replace and `$items...$id` resolution between two POSTs (dependencies first, then the rest). +> - **VariableLibrary value-set activation:** A post-deploy `PATCH /v1/workspaces/{ws}/variableLibraries/{id}` call sets the active value set per environment. +> +> Both are workarounds, not platform fixes. Orphan cleanup, the broader fabric-cicd feature surface (`key_value_replace`, `spark_pool`, `semantic_model_binding`), and per-item service principal coverage remain unimplemented in this repo's bulk path. If you go with bulk in your own project, you take on the same caller-side work. See the [Bulk CI/CD Implementation Guide](fabric-bulk-cicd-guide.md) for the full implementation walkthrough. + **When to choose fabric-cicd:** - You want environment-specific configuration handled declaratively - You need orphan cleanup as part of the deployment @@ -279,7 +286,7 @@ Both implementations sit inside Option 3 — branch per stage, build environment - You're moving large numbers of items and want fewer round trips - You need a workspace-level export (e.g., for disaster-recovery snapshots) that the Bulk Export API provides -Recommendation today: fabric-cicd. The Bulk APIs are an exciting direction and worth tracking, but in their current Preview state the missing parameterization layer and lack of orphan cleanup mean the caller has to rebuild capabilities the library already provides. Re-evaluate when (a) the APIs exit Preview and (b) Microsoft adds a parameterization story, or when your repo has already been refactored so that those capabilities aren't needed. +Recommendation today: fabric-cicd. The Bulk APIs are still in Preview and have no parameterization or orphan-cleanup story at the API level — the caller must implement substitution, value-set activation, and any delete logic themselves. fabric-cicd already provides these capabilities, maintained by Microsoft. This repo's bulk path shows that bridging is feasible and what it costs (~600 lines of Python + a config file), but choosing bulk means you own that bridging code. Re-evaluate when (a) the APIs exit Preview and (b) Microsoft adds parameterization and orphan-cleanup at the API level, or when your repo's shape doesn't need those capabilities to begin with. > This repository demonstrates both. The fabric-cicd workflows are the recommended path; the Bulk API workflows are included alongside them for evaluation. A `DEPLOY_METHOD` repository variable selects which implementation runs. See the [README quick start](README.md#quick-start) for how to switch. diff --git a/fabric-development-process.md b/fabric-development-process.md index 8b39c9f..10a0a8e 100644 --- a/fabric-development-process.md +++ b/fabric-development-process.md @@ -93,11 +93,13 @@ A Python script at `scripts/workspace_swap.py` handles the full lifecycle of fea The script automatically: - Detects the current branch name (no arguments needed). - Reads dev IDs from `variables.json` (the default value set). - - Reads feature workspace/lakehouse IDs from `.env` (or prompts if `.env` is missing). + - Reads feature workspace/lakehouse IDs from `.env`. If `.env` is missing or has empty/missing keys, the script exits with an error pointing at `.env.sample` — there is no interactive fallback. + - Displays the planned swap and asks `Type YES (uppercase) to apply, anything else to abort.` Type `YES` (case-sensitive) to proceed; the run is dry only when `--dry-run` is also passed. - Creates a feature branch value set (e.g., `valueSets/.json`). - Adds the value set to `settings.json`. - Rewrites the semantic model Direct Lake connection in `expressions.tmdl`. - Rewrites notebook META dependency blocks in all `notebook-content.py` files. + - Sweeps stale feature IDs from a previous swap if `.env` was changed since the last run (recovery pass). - Validates no dev IDs remain in critical files. 5. **Commit and push** the changes to the feature branch: ``` @@ -155,7 +157,7 @@ The repo ships slash commands in `.github/prompts/` that wrap the CLI. In Copilo - `/swap-to-dev-dryrun` — preview the revert without writing files - `/check-pr-ready` — run the CI-style readiness check locally -Copilot will execute the script in the VS Code integrated terminal and show you the output. This is useful when you are already working in Copilot Chat and want to stay in the same workflow without switching to the terminal. +Copilot will execute the script in the VS Code integrated terminal and show you the output. The `/swap-to-feature` slash command moves the `YES` confirmation into the chat UI — you click `YES` or `NO` in chat and Copilot pipes the answer to the script so the terminal never blocks. This is useful when you are already working in Copilot Chat and want to stay in the same workflow without switching to the terminal. Note: Copilot cannot auto-trigger the script on branch checkout. You still need to invoke a slash command or run it yourself after pulling a feature branch. @@ -170,7 +172,9 @@ Note: Copilot cannot auto-trigger the script on branch checkout. You still need 3. Paste both into `.env`. 4. Run `python scripts/workspace_swap.py` (or `/swap-to-feature` in Copilot Chat). -If `.env` is missing or empty, the script falls back to an interactive prompt. After the first run, the value set on disk is the source of truth for that branch — `.env` is no longer consulted for it. +If `.env` is missing, has empty values, or is missing either key, the script exits with a clear error pointing at `.env.sample`. There is no interactive fallback — `.env` is the single source of truth for swap-to-feature. + +For swap-to-feature, the script always reads `.env` (the existing value-set file does not override it). The value set on disk is read by swap-to-dev (to know which feature IDs to revert) and by the recovery pass (to detect previously-applied stale IDs that need rewriting). Before any rewrite happens, the script displays the planned dev → feature change and waits for the user to type literal `YES` to confirm. The script intentionally does **not** auto-discover IDs via the Fabric REST API. An earlier implementation matched workspaces by display name, which could silently pick the wrong workspace (e.g. matching the dev workspace itself), causing the swap to abort with no value set written. Explicit `.env` config avoids that class of bug. diff --git a/fabric-hybrid-cicd-guide.md b/fabric-hybrid-cicd-guide.md index 6c73391..f58bb67 100644 --- a/fabric-hybrid-cicd-guide.md +++ b/fabric-hybrid-cicd-guide.md @@ -30,8 +30,8 @@ Git repo (dev branch) ┌─────────────────────────────────────────────────────┐ │ deploy-test.yml (orchestrator) │ │ │ -│ deploy-supported │ -│ └─ reusable-deploy-supported.yml │ +│ deploy-fabric-cicd │ +│ └─ reusable-deploy-fabric-cicd.yml │ │ └─ fabric-cicd: publish_all_items() │ │ (Phase 1: Lakehouse + Ontology) │ │ (Phase 2: all remaining items) │ @@ -47,6 +47,8 @@ Git repo (dev branch) The same pattern applies to Prod (`deploy-prod.yml` → `etl-prod.yml`), triggered on push to `main`. +> An alternative bulk-API deploy path exists alongside this fabric-cicd path; both are gated by the `DEPLOY_METHOD` repo variable. The fabric-cicd path is the recommended one — see [fabric-cicd vs Bulk APIs](fabric-cicd-release-options.md#tooling-within-option-3-fabric-cicd-vs-bulk-apis) for the comparison and [Bulk CI/CD Implementation Guide](fabric-bulk-cicd-guide.md) for the bulk path's implementation walkthrough. + ### Branches & Workspaces | Branch | Workspace | Deployment Method | @@ -69,31 +71,41 @@ microsoft-fabric-sdlc-patterns/ │ │ ├── actions.instructions.md # Copilot instructions for workflow authoring │ │ └── python.instructions.md # Copilot instructions for Python scripts │ └── workflows/ -│ ├── deploy-test.yml # Orchestrator: push to test → deploy + ETL -│ ├── deploy-prod.yml # Orchestrator: push to main → deploy + ETL -│ ├── etl-test.yml # Triggers after deploy-test succeeds -│ ├── etl-prod.yml # Triggers after deploy-prod succeeds -│ ├── reusable-deploy-supported.yml # Template: fabric-cicd deployment -│ ├── reusable-fabric-etl.yml # Template: run Notebook via Fabric REST API -│ ├── check-pr-ready.yml # PR check: blocks feature IDs from merging to dev -│ ├── run-tests.yml # PR check: runs pytest when scripts/tests change -│ └── enforce-promotion-path.yml # PR check: enforces dev→test→main source-branch promotion +│ ├── deploy-test.yml # Orchestrator: push to test → fabric-cicd deploy +│ ├── deploy-prod.yml # Orchestrator: push to main → fabric-cicd deploy +│ ├── deploy-test-bulk.yml # Alternative orchestrator: bulk API deploy on push to test +│ ├── deploy-prod-bulk.yml # Alternative orchestrator: bulk API deploy on push to main +│ ├── etl-test.yml # Triggers after either deploy-test* succeeds +│ ├── etl-prod.yml # Triggers after either deploy-prod* succeeds +│ ├── reusable-deploy-fabric-cicd.yml # Template: fabric-cicd deployment +│ ├── reusable-deploy-bulk.yml # Template: Bulk Import API deployment (Preview) +│ ├── reusable-fabric-etl.yml # Template: run Notebook via Fabric REST API +│ ├── check-pr-ready.yml # PR check: blocks feature IDs from merging to dev +│ ├── run-tests.yml # PR check: runs pytest when scripts/tests change +│ └── enforce-promotion-path.yml # PR check: enforces dev→test→main source-branch promotion ├── data/ │ └── fabric/ # Fabric item definitions (repository_directory) │ ├── parameter.yml # fabric-cicd deploy-time parameterization -│ ├── PatternsLakehouse.Lakehouse/ # Lakehouse definition -│ ├── Patterns_Variables. # Variable Library with value sets -│ │ VariableLibrary/ -│ │ ├── variables.json # Default (Dev) variable values -│ │ ├── settings.json # Value set ordering +│ ├── bulk-parameter.yml # Bulk path's parameterization (independent format) +│ ├── PatternsLakehouse.Lakehouse/ +│ ├── Patterns_Ontology.Ontology/ +│ ├── Patterns_Variables.VariableLibrary/ +│ │ ├── variables.json +│ │ ├── settings.json │ │ └── valueSets/ -│ │ ├── Test.json # Test environment overrides -│ │ └── Prod.json # Prod environment overrides -│ ├── Import_Patterns_Data. # ETL notebook (creates Delta tables) -│ │ Notebook/ -│ └── Patterns_Patients_Data.Notebook/ # Query notebook (reads patients table) +│ │ ├── Test.json +│ │ └── Prod.json +│ ├── Import_Patterns_Data.Notebook/ # ETL notebook (creates Delta tables) +│ ├── Patterns_Patients_Data.Notebook/ +│ ├── Patterns_Demo.Notebook/ +│ ├── Patterns_Semantic_Model.SemanticModel/ +│ ├── Patterns_Report.Report/ +│ └── Patterns_Data_Agent.DataAgent/ ├── scripts/ -│ └── workspace_swap.py # Bootstrap/reset feature branch workspace bindings +│ ├── workspace_swap.py # Bootstrap/reset feature branch workspace bindings +│ ├── deploy_fabric_cicd.py # fabric-cicd deploy (invoked by reusable-deploy-fabric-cicd.yml) +│ ├── deploy_bulk.py # Bulk Import API deploy (invoked by reusable-deploy-bulk.yml) +│ └── run_fabric_etl.py # Run a Fabric Notebook job (invoked by reusable-fabric-etl.yml) ├── assets/ # Architecture diagrams (SVG) ├── fabric-cicd-release-options.md # CI/CD strategy and release option comparison ├── fabric-hybrid-cicd-guide.md # This file @@ -116,7 +128,7 @@ microsoft-fabric-sdlc-patterns/ ### Deploy Job -Each deploy workflow calls `reusable-deploy-supported.yml`, which publishes all supported items from Git to the target workspace using fabric-cicd. It uses a two-phase approach: Phase 1 deploys Lakehouse + Ontology, Phase 2 deploys all remaining items (Variable Library, Notebooks, Semantic Model, Report, Data Agent). Item types are explicitly scoped via `item_type_in_scope`. +Each deploy workflow calls `reusable-deploy-fabric-cicd.yml`, which publishes all supported items from Git to the target workspace using fabric-cicd. It uses a two-phase approach: Phase 1 deploys Lakehouse + Ontology, Phase 2 deploys all remaining items (Variable Library, Notebooks, Semantic Model, Report, Data Agent). Item types are explicitly scoped via `item_type_in_scope`. The ETL workflow triggers automatically after the deploy workflow completes successfully. If the deploy fails, ETL does not run. @@ -130,7 +142,7 @@ The ETL workflow triggers automatically after the deploy workflow completes succ | Template | Purpose | |---|---| -| `reusable-deploy-supported.yml` | Two-phase fabric-cicd deployment: Phase 1 deploys Lakehouse + Ontology, Phase 2 deploys all remaining items via `publish_all_items()` and `unpublish_all_orphan_items()`. Accepts `environment`, `repository_directory`, and optional `item_type_in_scope` inputs. | +| `reusable-deploy-fabric-cicd.yml` | Two-phase fabric-cicd deployment: Phase 1 deploys Lakehouse + Ontology, Phase 2 deploys all remaining items via `publish_all_items()` and `unpublish_all_orphan_items()`. Accepts `environment`, `repository_directory`, and optional `item_type_in_scope` inputs. | | `reusable-fabric-etl.yml` | Resolves a Fabric item by **name** (not ID) via the List Items API, then starts a job (RunNotebook) and polls until completion. No item IDs need to be known ahead of time. | ### Why Reusable Workflows (Not Composite Actions) @@ -286,7 +298,7 @@ Confirm all items are functional in the target workspace: The Variable Library and Semantic Model need the lakehouse ID for each environment, but the lakehouse doesn't exist in Test/Prod until the first deployment creates it. fabric-cicd's `$items` dynamic variables (e.g., `$items.Lakehouse.PatternsLakehouse.$id`) resolve by querying the **live target workspace** during parameterization — before items are published. On the first deployment to an empty workspace, this query returns nothing and parameterization fails. -**Solution:** The `reusable-deploy-supported.yml` workflow uses a **two-phase deployment** approach. Phase 1 calls `publish_all_items()` with `item_type_in_scope=["Lakehouse", "Ontology"]` to create the Lakehouse and Ontology first. The Lakehouse must exist so that `$items.Lakehouse.PatternsLakehouse.$id` resolves for parameter.yml rules. The Ontology must exist so that the Data Agent's logicalId reference resolves (fabric-cicd caches workspace state once per `publish_all_items()` call, so items deployed within the same call aren't visible to later items' logicalId resolution). Phase 2 calls `publish_all_items()` with the remaining item types. On subsequent deployments, both phases are idempotent. +**Solution:** The `reusable-deploy-fabric-cicd.yml` workflow uses a **two-phase deployment** approach. Phase 1 calls `publish_all_items()` with `item_type_in_scope=["Lakehouse", "Ontology"]` to create the Lakehouse and Ontology first. The Lakehouse must exist so that `$items.Lakehouse.PatternsLakehouse.$id` resolves for parameter.yml rules. The Ontology must exist so that the Data Agent's logicalId reference resolves (fabric-cicd caches workspace state once per `publish_all_items()` call, so items deployed within the same call aren't visible to later items' logicalId resolution). Phase 2 calls `publish_all_items()` with the remaining item types. On subsequent deployments, both phases are idempotent. ### Item Type Scoping diff --git a/scripts/deploy_bulk.py b/scripts/deploy_bulk.py index ecfa643..7d4831d 100644 --- a/scripts/deploy_bulk.py +++ b/scripts/deploy_bulk.py @@ -20,9 +20,22 @@ (DEPENDENCY_TYPES) so their IDs become available, then substitute those IDs into the remaining items' definitions and POST the rest. +The Bulk Import API itself has no parameterization, no VariableLibrary +value-set activation, and no delete support. fabric-cicd handles the +first two automatically in its library; with the bulk API those become +caller responsibilities. This script implements substitution (driven by +bulk-parameter.yml) and value-set activation (a post-deploy PATCH call) +so the demo repo works end-to-end. They are workarounds, not platform +fixes — choosing bulk in your own project means owning equivalent code. + Known gaps vs deploy_fabric_cicd.py (intentional, documented): -- No orphan cleanup (Bulk Import API only supports Create/Update, not Delete) -- No item_type_in_scope filter (deploys everything in repository_directory) +- No full parameter.yml feature coverage. bulk-parameter.yml supports + find_replace + $items + $workspace + $environment only. fabric-cicd's + key_value_replace, spark_pool, semantic_model_binding are not + implemented here. +- No orphan cleanup. Bulk Import API only supports Create/Update, + not Delete. +- No item_type_in_scope filter (deploys everything in repository_directory). API references: - Bulk import: https://learn.microsoft.com/en-us/rest/api/fabric/core/items/bulk-import-item-definitions(beta) @@ -42,6 +55,7 @@ import sys import time from dataclasses import dataclass, field +from typing import TypedDict import requests import yaml @@ -49,6 +63,7 @@ # Polling configuration POLL_FALLBACK_SECONDS = 30 POLL_FLOOR_SECONDS = 5 +POLL_CEILING_SECONDS = 600 POLL_TIMEOUT_SECONDS = 20 * 60 TOKEN_REFRESH_EVERY_N_POLLS = 20 @@ -87,6 +102,53 @@ _ENVIRONMENT_PLACEHOLDER = "$environment" +# ----- TypedDicts for the Fabric Bulk Import API surface --------------------- +# +# These document the shapes the script sends and receives. They're not +# enforced at runtime (TypedDict is purely a type hint) but they give +# Pylance/mypy the information needed to catch field-name typos and to +# autocomplete on response objects. + + +class DefinitionPart(TypedDict): + """One element of the request's ``definitionParts`` array. + + All three fields are required by the Bulk Import API. ``payload`` is + base64-encoded file content; ``payloadType`` is always ``"InlineBase64"`` + in this script. + """ + + path: str + payload: str + payloadType: str + + +class ImportItemDetail(TypedDict, total=False): + """One element of ``importItemDefinitionsDetails`` in a bulk-import response. + + Marked ``total=False`` because the API does not always return every + field on every entry (e.g., a partial-failure entry may omit ``itemId``). + """ + + itemDisplayName: str + itemType: str + itemId: str + operationStatus: str + + +class BulkResponseBody(TypedDict, total=False): + """The shape of a sync 200 body or an LRO ``/result`` body.""" + + importItemDefinitionsDetails: list[ImportItemDetail] + + +class LROStatusBody(TypedDict, total=False): + """Body of GET ``/v1/operations/{id}`` while polling.""" + + status: str + failureReason: str + + @dataclass(frozen=True) class SubstitutionRule: """A single find/replace rule scoped to one or more item types. @@ -102,17 +164,29 @@ class SubstitutionRule: item_types: frozenset[str] +@dataclass(frozen=True) +class VariableLibraryConfig: + """VariableLibrary-related settings parsed from bulk-parameter.yml. + + A separate dataclass (rather than a flat field on BulkConfig) so future + VariableLibrary settings can be added here without changing the shape + of the parent config. + """ + + active_value_set: str | None = None + + @dataclass(frozen=True) class BulkConfig: """Parsed contents of bulk-parameter.yml. - ``variable_library_active_value_set`` is None when the config has no + ``variable_library.active_value_set`` is None when the config has no ``variable_library`` block or when its ``active_value_set`` is null. In that case the deploy skips the value-set activation step. """ substitutions: tuple[SubstitutionRule, ...] = field(default_factory=tuple) - variable_library_active_value_set: str | None = None + variable_library: VariableLibraryConfig = field(default_factory=VariableLibraryConfig) def load_bulk_config(path: pathlib.Path) -> BulkConfig: @@ -178,7 +252,7 @@ def load_bulk_config(path: pathlib.Path) -> BulkConfig: return BulkConfig( substitutions=tuple(substitutions), - variable_library_active_value_set=active_value_set, + variable_library=VariableLibraryConfig(active_value_set=active_value_set), ) @@ -216,7 +290,9 @@ def item_display_name_of(part_path: str) -> str | None: return folder.rsplit(".", 1)[0] -def partition_dependencies(parts: list[dict]) -> tuple[list[dict], list[dict]]: +def partition_dependencies( + parts: list[DefinitionPart], +) -> tuple[list[DefinitionPart], list[DefinitionPart]]: """Split definitionParts[] into (dependencies, remaining). A part belongs to ``dependencies`` if its item type is in @@ -224,8 +300,8 @@ def partition_dependencies(parts: list[dict]) -> tuple[list[dict], list[dict]]: their IDs are available to resolve ``$items...$id`` placeholders in the remaining items' substitution rules. """ - dependencies: list[dict] = [] - remaining: list[dict] = [] + dependencies: list[DefinitionPart] = [] + remaining: list[DefinitionPart] = [] for part in parts: if item_type_of(part["path"]) in DEPENDENCY_TYPES: dependencies.append(part) @@ -234,7 +310,7 @@ def partition_dependencies(parts: list[dict]) -> tuple[list[dict], list[dict]]: return dependencies, remaining -def extract_item_ids(response_body: dict) -> dict[tuple[str, str], str]: +def extract_item_ids(response_body: BulkResponseBody) -> dict[tuple[str, str], str]: """Build a lookup of deployed item IDs from a bulk-import response body. Returns ``{(item_type, item_display_name): item_id}``. Skips entries @@ -282,11 +358,11 @@ def _replace_items(match: re.Match[str]) -> str: def apply_substitutions( - parts: list[dict], + parts: list[DefinitionPart], rules: tuple[SubstitutionRule, ...], workspace_id: str, item_id_map: dict[tuple[str, str], str], -) -> list[dict]: +) -> list[DefinitionPart]: """Apply all substitution rules to the matching definitionParts[]. For each part: @@ -304,7 +380,7 @@ def apply_substitutions( if not rules: return parts - out: list[dict] = [] + out: list[DefinitionPart] = [] for part in parts: path = part["path"] item_type = item_type_of(path) @@ -332,8 +408,11 @@ def apply_substitutions( out.append(part) continue - new_part = dict(part) - new_part["payload"] = base64.b64encode(modified_text.encode("utf-8")).decode("ascii") + new_part: DefinitionPart = { + "path": part["path"], + "payload": base64.b64encode(modified_text.encode("utf-8")).decode("ascii"), + "payloadType": part["payloadType"], + } out.append(new_part) return out @@ -384,16 +463,13 @@ def acquire_token(tenant_id: str, client_id: str, client_secret: str) -> str: ) if resp.status_code != 200: sys.exit(f"::error::Token acquisition failed: HTTP {resp.status_code} {resp.text}") - token = resp.json()["access_token"] - # Mask the token in workflow logs - print(f"::add-mask::{token}") - return token + return resp.json()["access_token"] -def build_definition_parts(repo_dir: pathlib.Path) -> list[dict]: +def build_definition_parts(repo_dir: pathlib.Path) -> list[DefinitionPart]: if not repo_dir.is_dir(): sys.exit(f"::error::Repository directory not found: {repo_dir}") - parts: list[dict] = [] + parts: list[DefinitionPart] = [] for f in sorted(repo_dir.rglob("*")): if not f.is_file(): continue @@ -414,6 +490,24 @@ def build_definition_parts(repo_dir: pathlib.Path) -> list[dict]: return parts +def _parse_retry_after(raw: str | None) -> int: + """Convert a Retry-After header value into a clamped integer. + + Returns the parsed value clamped to ``[POLL_FLOOR_SECONDS, POLL_CEILING_SECONDS]`` + on success. Falls back to ``POLL_FALLBACK_SECONDS`` (also clamped) when the + header is missing or unparseable. Clamping to a ceiling prevents a malformed + or pathological response from sleeping past the global polling timeout. + """ + if raw is None: + value = POLL_FALLBACK_SECONDS + else: + try: + value = int(raw) + except (TypeError, ValueError): + value = POLL_FALLBACK_SECONDS + return max(POLL_FLOOR_SECONDS, min(value, POLL_CEILING_SECONDS)) + + def poll_lro( operation_id: str, headers: dict, @@ -423,7 +517,7 @@ def poll_lro( client_secret: str, ) -> None: base = "https://api.fabric.microsoft.com/v1/operations" - retry_after = max(initial_retry_after or POLL_FALLBACK_SECONDS, POLL_FLOOR_SECONDS) + retry_after = _parse_retry_after(str(initial_retry_after) if initial_retry_after else None) started = time.monotonic() poll_count = 0 @@ -447,7 +541,7 @@ def poll_lro( if resp.status_code != 200: sys.exit(f"::error::Poll request failed: HTTP {resp.status_code} {resp.text}") - body = resp.json() + body: LROStatusBody = resp.json() status = body.get("status", "Unknown") print(f"Poll {poll_count} (t+{int(elapsed)}s): status={status}") @@ -457,14 +551,12 @@ def poll_lro( print(json.dumps(body, indent=2)) sys.exit(f"::error::LRO ended with status: {status}") - # NotStarted or Running — keep polling. Honor Retry-After if present. - retry_after = max( - int(resp.headers.get("Retry-After", POLL_FALLBACK_SECONDS)), - POLL_FLOOR_SECONDS, - ) + # NotStarted or Running — keep polling. Honor Retry-After if present + # but clamp it so a pathological value can't bypass the global timeout. + retry_after = _parse_retry_after(resp.headers.get("Retry-After")) -def check_per_item_status(result: dict) -> None: +def check_per_item_status(result: BulkResponseBody) -> None: details = result.get("importItemDefinitionsDetails", []) print(json.dumps(result, indent=2)) if not details: @@ -487,7 +579,7 @@ def check_per_item_status(result: dict) -> None: def interpret_post_response( status_code: int, - body: dict, + body: BulkResponseBody, headers: dict, ) -> tuple[str, ...]: """Pure decision function for a bulk-import POST response. @@ -511,7 +603,7 @@ def interpret_post_response( operation_id = headers.get("x-ms-operation-id") if not operation_id: return ("missing_op_id",) - retry_after = int(headers.get("Retry-After", POLL_FALLBACK_SECONDS)) + retry_after = _parse_retry_after(headers.get("Retry-After")) return ("async", operation_id, retry_after) return ("error", status_code) @@ -549,14 +641,14 @@ def activate_variable_library_value_set( def post_bulk( - parts: list[dict], + parts: list[DefinitionPart], workspace_id: str, headers: dict, tenant_id: str, client_id: str, client_secret: str, label: str, -) -> dict: +) -> BulkResponseBody: """POST one bulkImportDefinitions request and return the result body. Handles both the synchronous (200) and asynchronous (202 + LRO) response @@ -628,11 +720,22 @@ def main() -> None: parts = build_definition_parts(repo_dir) print(f"Built {len(parts)} definition parts from {repo_dir}") - config = load_bulk_config(repo_dir / BULK_PARAMETER_FILENAME) + config_path = repo_dir / BULK_PARAMETER_FILENAME + config = load_bulk_config(config_path) if config.substitutions: - print(f"Loaded {len(config.substitutions)} substitution rule(s) from {BULK_PARAMETER_FILENAME}") + print( + f"Loaded {len(config.substitutions)} substitution rule(s) from {config_path}" + ) + elif not config_path.exists(): + print( + f"No {BULK_PARAMETER_FILENAME} at {config_path}; " + f"bulk deploy will use a single POST with no substitutions" + ) else: - print(f"No substitution rules found (no {BULK_PARAMETER_FILENAME} or empty substitutions)") + print( + f"{config_path} exists but defines no substitution rules; " + f"bulk deploy will use a single POST with no substitutions" + ) # Decide single-deploy vs. two-deploy. We only need to deploy the # dependency types separately when at least one rule references their @@ -682,7 +785,7 @@ def main() -> None: # equivalent of fabric-cicd's environment-driven value-set selection, so # we make the PATCH call ourselves. active_value_set = resolve_active_value_set( - config.variable_library_active_value_set, environment + config.variable_library.active_value_set, environment ) if active_value_set: library_id = find_variable_library_id(item_id_map) diff --git a/scripts/deploy_fabric_cicd.py b/scripts/deploy_fabric_cicd.py index 1efc69b..39d6d33 100644 --- a/scripts/deploy_fabric_cicd.py +++ b/scripts/deploy_fabric_cicd.py @@ -13,7 +13,7 @@ deployments all items already exist and phases are idempotent — they simply update in place. -Invoked by .github/workflows/reusable-deploy-supported.yml. +Invoked by .github/workflows/reusable-deploy-fabric-cicd.yml. Required environment variables: AZURE_TENANT_ID, AZURE_CLIENT_ID, AZURE_CLIENT_SECRET, diff --git a/scripts/run_fabric_etl.py b/scripts/run_fabric_etl.py index 79fc3d5..8ce5420 100644 --- a/scripts/run_fabric_etl.py +++ b/scripts/run_fabric_etl.py @@ -23,12 +23,39 @@ import os import sys import time +from typing import TypedDict import requests from azure.identity import ClientSecretCredential -def find_item_id_by_name(items: list[dict], item_name: str) -> str: +# ----- TypedDicts for the Fabric Items + Jobs API surface -------------------- +# +# Document the response shapes the script consumes. Not enforced at runtime +# (TypedDict is purely a type hint) but gives Pylance/mypy the information +# needed to catch field-name typos. + + +class FabricItem(TypedDict, total=False): + """One element of the List Items API ``value`` array. + + Marked ``total=False`` because the API returns additional fields + (description, workspaceId, etc.) we don't consume here. + """ + + id: str + displayName: str + type: str + + +class JobStatusBody(TypedDict, total=False): + """Body of a Fabric Jobs API status poll response.""" + + status: str + failureReason: str + + +def find_item_id_by_name(items: list[FabricItem], item_name: str) -> str: """Return the ID of the item whose displayName matches item_name. Exits with code 1 if no match is found, printing the available item names @@ -46,7 +73,7 @@ def find_item_id_by_name(items: list[dict], item_name: str) -> str: def interpret_poll_response( status_code: int, - body: dict, + body: JobStatusBody, headers: dict, ) -> tuple[str, ...]: """Pure decision function for a job-status poll response. @@ -103,7 +130,7 @@ def main() -> None: print(f"Failed to list items: {list_response.status_code} {list_response.text}") sys.exit(1) - items = list_response.json().get("value", []) + items: list[FabricItem] = list_response.json().get("value", []) item_id = find_item_id_by_name(items, item_name) print(f"Resolved {item_name} -> {item_id}") diff --git a/scripts/workspace_swap.py b/scripts/workspace_swap.py index dc6eb25..f9f6fbf 100644 --- a/scripts/workspace_swap.py +++ b/scripts/workspace_swap.py @@ -36,7 +36,16 @@ import subprocess import sys from pathlib import Path -from typing import Callable +from typing import Callable, TypedDict + +# Force UTF-8 for stdout/stderr so the Unicode box-drawing characters used +# in section headers and the summary banner don't crash on Windows consoles +# that default to cp1252. ``errors="replace"`` keeps the script running if +# any output sneaks through that the codec still can't handle. +for _stream in (sys.stdout, sys.stderr): + reconfigure = getattr(_stream, "reconfigure", None) + if reconfigure is not None: + reconfigure(encoding="utf-8", errors="replace") # ── Paths (relative to repo root) ────────────────────────────────────────── REPO_ROOT = Path(subprocess.run( @@ -61,7 +70,17 @@ # needs_rewrite – True → dev↔feature ID replacement during bootstrap/reset # id_keys – which dev IDs to validate ("workspace", "lakehouse", or both) # content_filter – optional predicate on file text; None means process all -ITEM_TYPES: list[dict[str, str | list[str] | bool | Callable[[str], bool] | None]] = [ +class ItemTypeRegistryEntry(TypedDict): + """One entry in ``ITEM_TYPES``. See module-level comment for field meanings.""" + + name: str + file_patterns: list[str] + needs_rewrite: bool + id_keys: list[str] + content_filter: Callable[[str], bool] | None + + +ITEM_TYPES: list[ItemTypeRegistryEntry] = [ { "name": "SemanticModel", "file_patterns": ["*.SemanticModel/definition/expressions.tmdl"], @@ -156,43 +175,86 @@ def get_dev_ids() -> tuple[str, str]: def resolve_feature_ids(branch: str, value_set_path: Path) -> tuple[str, str]: """ - Resolve workspace and lakehouse IDs for the feature environment. + Resolve workspace and lakehouse IDs for the feature environment from .env. + + .env is the single source of truth for swap-to-feature. The script does NOT + fall back to the existing value-set file or to interactive input — both were + removed because they let stale values silently override what the developer + typed in .env, producing swaps that pointed at the wrong workspace. + + The user is asked to confirm the resolved IDs in _run_swap_to_feature + before any files are rewritten. - Priority: - 1. Existing value set file for this branch (already bootstrapped). - 2. .env file at repo root (FEATURE_WORKSPACE_ID, FEATURE_LAKEHOUSE_ID). - 3. Interactive prompt as fallback. + Exits with a clear error when .env is missing or either key is missing or + blank. """ - # 1. Reuse existing value set - if value_set_path.exists(): - overrides = load_json(value_set_path)["variableOverrides"] - lookup = {o["name"]: o["value"] for o in overrides} - ws_id = lookup.get("target_workspace_id") - lh_id = lookup.get("target_lakehouse_id") - if ws_id and lh_id: - print(f" Reusing IDs from existing value set: {value_set_path.name}") - return ws_id, lh_id - - # 2. Try .env file at the repo root + # Note: ``value_set_path`` is unused but kept in the signature so callers + # don't have to change. Removing it would also break the test fixture's + # call shape. + _ = value_set_path + + if not ENV_FILE.exists(): + sys.exit( + f"ERROR: {ENV_FILE.name} not found at the repo root. " + f"Copy .env.sample to .env and fill in FEATURE_WORKSPACE_ID and " + f"FEATURE_LAKEHOUSE_ID for branch '{branch}'." + ) + env = _read_env_file(ENV_FILE) ws_id = env.get("FEATURE_WORKSPACE_ID", "").strip() lh_id = env.get("FEATURE_LAKEHOUSE_ID", "").strip() - if ws_id and lh_id: - _validate_guid(ws_id, "FEATURE_WORKSPACE_ID") - _validate_guid(lh_id, "FEATURE_LAKEHOUSE_ID") - print(f" Loaded feature IDs from {ENV_FILE.name}") - return ws_id, lh_id - - # 3. Interactive fallback - print(f"\nNo feature IDs found for branch '{branch}'.") - print("Tip: copy .env.sample to .env and fill in the GUIDs to skip this prompt.") - ws_id = input(" FEATURE_WORKSPACE_ID : ").strip() - lh_id = input(" FEATURE_LAKEHOUSE_ID : ").strip() + + missing: list[str] = [] + if not ws_id: + missing.append("FEATURE_WORKSPACE_ID") + if not lh_id: + missing.append("FEATURE_LAKEHOUSE_ID") + if missing: + sys.exit( + f"ERROR: {ENV_FILE.name} is missing or has empty values for: " + f"{', '.join(missing)}. See .env.sample for the expected shape." + ) + _validate_guid(ws_id, "FEATURE_WORKSPACE_ID") _validate_guid(lh_id, "FEATURE_LAKEHOUSE_ID") + print(f" Loaded feature IDs from {ENV_FILE.name}") return ws_id, lh_id +def _confirm_swap_to_feature( + branch: str, + dev_ws_id: str, + dev_lh_id: str, + new_ws_id: str, + new_lh_id: str, +) -> None: + """Show the planned swap and require the user to type ``YES`` to proceed. + + The swap rewrites tracked Fabric files. A previous bug surfaced when stale + .env values silently produced a swap pointed at the wrong workspace; this + confirmation gate forces the developer to look at the actual GUIDs before + any files change. Case-sensitive ``YES`` is required — ``yes``, ``y``, or + a blank line all abort. + + Skipped when the script is invoked with --dry-run (the dry-run is itself + the verification step). + """ + print("\nPlanned swap:") + print(f" Branch : {branch}") + print(f" Workspace ID : {dev_ws_id} → {new_ws_id}") + print(f" Lakehouse ID : {dev_lh_id} → {new_lh_id}") + print( + "\nThis will rewrite tracked Fabric files and create a per-branch " + "value set.\nType YES (uppercase) to apply, anything else to abort." + ) + try: + answer = input("Confirm: ").strip() + except EOFError: + sys.exit("Aborted (no input available).") + if answer != "YES": + sys.exit("Aborted by user.") + + def _validate_guid(value: str, name: str) -> None: pattern = re.compile(r"^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$") if not pattern.match(value): @@ -257,6 +319,29 @@ def repoint_items(dev_ws_id: str, dev_lh_id: str, return changed +def _read_previous_feature_ids( + value_set_path: Path, +) -> tuple[str | None, str | None]: + """Read previously-applied feature IDs from the per-branch value-set file. + + Returns ``(None, None)`` when the file doesn't exist (first swap on this + branch) or when either override is missing. + + Used to recover from a wrong-IDs-applied state: if a previous swap wrote + the wrong feature GUIDs into the SemanticModel/Notebook files, the value + set still records what was applied, so the next swap can find/replace + those stale GUIDs as well as the dev baseline. + """ + if not value_set_path.exists(): + return None, None + try: + overrides = load_json(value_set_path).get("variableOverrides", []) + except (json.JSONDecodeError, KeyError): + return None, None + lookup = {o["name"]: o["value"] for o in overrides if "name" in o and "value" in o} + return lookup.get("target_workspace_id"), lookup.get("target_lakehouse_id") + + def validate_no_ids(ws_id: str, lh_id: str, *, label: str = "target") -> list[str]: """Scan all registered item types for leftover IDs that should not be present.""" id_map = {"workspace": ws_id, "lakehouse": lh_id} @@ -384,6 +469,14 @@ def _run_swap_to_feature(branch: str, branch_label: str, value_set_path: Path, if new_ws_id == dev_ws_id and new_lh_id == dev_lh_id: sys.exit("ERROR: Feature IDs are identical to dev IDs. Nothing to do.") + # Capture the previously-applied feature IDs (if any) BEFORE the value set + # gets rewritten in step 3. Used in step 4 to recover from a prior swap + # that applied wrong IDs. + stale_ws_id, stale_lh_id = _read_previous_feature_ids(value_set_path) + + if not dry_run: + _confirm_swap_to_feature(branch, dev_ws_id, dev_lh_id, new_ws_id, new_lh_id) + changes: list[str] = [] print("\n3. Creating/updating value set...") @@ -393,10 +486,26 @@ def _run_swap_to_feature(branch: str, branch_label: str, value_set_path: Path, changes.append(f"Settings: {SETTINGS_FILE.relative_to(REPO_ROOT)}") print("\n4. Repointing items...") + # Standard pass: dev IDs -> new IDs. Handles the first-time swap and + # any items that still reference the dev baseline. repointed = repoint_items(dev_ws_id, dev_lh_id, new_ws_id, new_lh_id, dry_run=dry_run) for r in repointed: changes.append(f"Repointed: {r}") + # Recovery pass: if a previous run applied stale feature IDs that aren't + # in .env anymore, sweep them out too. Skip when the previous and target + # IDs match (no recovery needed) or when there's no previous record. + if stale_ws_id and stale_lh_id and ( + stale_ws_id != new_ws_id or stale_lh_id != new_lh_id + ): + print( + f" Recovery: also rewriting any stale feature IDs from prior swap " + f"({stale_ws_id} → {new_ws_id}; {stale_lh_id} → {new_lh_id})" + ) + recovered = repoint_items(stale_ws_id, stale_lh_id, new_ws_id, new_lh_id, dry_run=dry_run) + for r in recovered: + changes.append(f"Recovered: {r}") + print("\n5. Validating...") if dry_run: print(" [dry-run] Skipping validation (files unchanged).") diff --git a/tests/test_deploy_bulk.py b/tests/test_deploy_bulk.py index 58a443c..598b5c0 100644 --- a/tests/test_deploy_bulk.py +++ b/tests/test_deploy_bulk.py @@ -20,6 +20,7 @@ SUBSTITUTABLE_EXTENSIONS, BulkConfig, SubstitutionRule, + VariableLibraryConfig, acquire_token, apply_substitutions, build_definition_parts, @@ -140,7 +141,8 @@ def test_load_bulk_config_missing_file_returns_empty(tmp_path: pathlib.Path) -> config = load_bulk_config(tmp_path / "does_not_exist.yml") assert config == BulkConfig() assert config.substitutions == () - assert config.variable_library_active_value_set is None + assert config.variable_library == VariableLibraryConfig() + assert config.variable_library.active_value_set is None def test_load_bulk_config_empty_file_returns_empty(tmp_path: pathlib.Path) -> None: @@ -179,7 +181,7 @@ def test_load_bulk_config_full_happy_path(tmp_path: pathlib.Path) -> None: replace_with="$items.Lakehouse.LH.$id", item_types=frozenset({"Notebook"}), ) - assert config.variable_library_active_value_set == "$environment" + assert config.variable_library.active_value_set == "$environment" def test_load_bulk_config_substitutions_only(tmp_path: pathlib.Path) -> None: @@ -195,7 +197,7 @@ def test_load_bulk_config_substitutions_only(tmp_path: pathlib.Path) -> None: ) config = load_bulk_config(path) assert len(config.substitutions) == 1 - assert config.variable_library_active_value_set is None + assert config.variable_library.active_value_set is None def test_load_bulk_config_variable_library_only(tmp_path: pathlib.Path) -> None: @@ -209,7 +211,7 @@ def test_load_bulk_config_variable_library_only(tmp_path: pathlib.Path) -> None: ) config = load_bulk_config(path) assert config.substitutions == () - assert config.variable_library_active_value_set == "Test" + assert config.variable_library.active_value_set == "Test" def test_load_bulk_config_variable_library_null_active_value_set( @@ -224,7 +226,7 @@ def test_load_bulk_config_variable_library_null_active_value_set( """, encoding="utf-8", ) - assert load_bulk_config(path).variable_library_active_value_set is None + assert load_bulk_config(path).variable_library.active_value_set is None def test_load_bulk_config_top_level_not_a_mapping_raises(tmp_path: pathlib.Path) -> None: @@ -309,7 +311,7 @@ def test_load_bulk_config_real_repo_file_parses() -> None: config = load_bulk_config(path) assert isinstance(config, BulkConfig) assert len(config.substitutions) >= 1 - assert config.variable_library_active_value_set is not None + assert config.variable_library.active_value_set is not None # ---------- check_per_item_status ---------- @@ -395,6 +397,51 @@ def test_interpret_post_response_unexpected_status() -> None: assert action == ("error", 500) +def test_interpret_post_response_clamps_pathological_retry_after() -> None: + """A bogus Retry-After value must not bypass the global polling timeout.""" + action = interpret_post_response(202, {}, {"x-ms-operation-id": "op", "Retry-After": "999999"}) + assert action[0] == "async" + # Clamped to POLL_CEILING_SECONDS (600). + assert action[2] == 600 + + +def test_interpret_post_response_unparseable_retry_after_falls_back() -> None: + """A non-integer Retry-After header falls back to the default, doesn't crash.""" + action = interpret_post_response(202, {}, {"x-ms-operation-id": "op", "Retry-After": "bananas"}) + assert action[0] == "async" + # Falls back to POLL_FALLBACK_SECONDS (30). + assert action[2] == 30 + + +# ---------- _parse_retry_after ---------- + + +def test_parse_retry_after_normal_value() -> None: + from deploy_bulk import _parse_retry_after + assert _parse_retry_after("45") == 45 + + +def test_parse_retry_after_none_returns_fallback() -> None: + from deploy_bulk import POLL_FALLBACK_SECONDS, _parse_retry_after + assert _parse_retry_after(None) == POLL_FALLBACK_SECONDS + + +def test_parse_retry_after_invalid_returns_fallback() -> None: + from deploy_bulk import POLL_FALLBACK_SECONDS, _parse_retry_after + assert _parse_retry_after("not-a-number") == POLL_FALLBACK_SECONDS + + +def test_parse_retry_after_clamps_to_floor() -> None: + from deploy_bulk import POLL_FLOOR_SECONDS, _parse_retry_after + assert _parse_retry_after("0") == POLL_FLOOR_SECONDS + assert _parse_retry_after("-100") == POLL_FLOOR_SECONDS + + +def test_parse_retry_after_clamps_to_ceiling() -> None: + from deploy_bulk import POLL_CEILING_SECONDS, _parse_retry_after + assert _parse_retry_after("999999") == POLL_CEILING_SECONDS + + # ---------- acquire_token ---------- @@ -409,8 +456,10 @@ def test_acquire_token_success(capsys: pytest.CaptureFixture) -> None: call_kwargs = post.call_args.kwargs assert call_kwargs["data"]["grant_type"] == "client_credentials" assert call_kwargs["data"]["scope"] == "https://api.fabric.microsoft.com/.default" - # Workflow log mask was emitted - assert "::add-mask::fake-token-xyz" in capsys.readouterr().out + # The token must NOT appear in stdout. Earlier versions of this script + # emitted an `::add-mask::` workflow command; that line itself + # leaked the token before GitHub's mask filter could redact it. + assert "fake-token-xyz" not in capsys.readouterr().out def test_acquire_token_failure_exits() -> None: @@ -726,43 +775,43 @@ def test_find_variable_library_id_multiple_raises() -> None: } with pytest.raises(ValueError, match="Expected exactly one VariableLibrary"): find_variable_library_id(item_map) - - -# ---------- activate_variable_library_value_set ---------- - - -def test_activate_variable_library_value_set_happy_path( - capsys: pytest.CaptureFixture, -) -> None: - fake_resp = mock.Mock(status_code=200, text="{}") - headers = {"Authorization": "Bearer x", "Content-Type": "application/json"} - with mock.patch("deploy_bulk.requests.patch", return_value=fake_resp) as patch: - from deploy_bulk import activate_variable_library_value_set - activate_variable_library_value_set( - workspace_id="ws-id", library_id="vl-id", - value_set_name="Test", headers=headers, - ) - patch.assert_called_once() - call_args = patch.call_args - url = call_args.args[0] - assert url.endswith("/v1/workspaces/ws-id/variableLibraries/vl-id") - body = call_args.kwargs["json"] - assert body == {"properties": {"activeValueSetName": "Test"}} - assert call_args.kwargs["headers"] is headers - out = capsys.readouterr().out - assert "Test" in out - assert "vl-id" in out - - -def test_activate_variable_library_value_set_failure_exits() -> None: - fake_resp = mock.Mock(status_code=400, text="Bad value set name") - headers = {"Authorization": "Bearer x"} - with mock.patch("deploy_bulk.requests.patch", return_value=fake_resp): - from deploy_bulk import activate_variable_library_value_set - with pytest.raises(SystemExit) as exc: - activate_variable_library_value_set( - workspace_id="ws", library_id="vl", - value_set_name="Bad", headers=headers, - ) - assert "Failed to set active value set" in str(exc.value) - assert "400" in str(exc.value) + + +# ---------- activate_variable_library_value_set ---------- + + +def test_activate_variable_library_value_set_happy_path( + capsys: pytest.CaptureFixture, +) -> None: + fake_resp = mock.Mock(status_code=200, text="{}") + headers = {"Authorization": "Bearer x", "Content-Type": "application/json"} + with mock.patch("deploy_bulk.requests.patch", return_value=fake_resp) as patch: + from deploy_bulk import activate_variable_library_value_set + activate_variable_library_value_set( + workspace_id="ws-id", library_id="vl-id", + value_set_name="Test", headers=headers, + ) + patch.assert_called_once() + call_args = patch.call_args + url = call_args.args[0] + assert url.endswith("/v1/workspaces/ws-id/variableLibraries/vl-id") + body = call_args.kwargs["json"] + assert body == {"properties": {"activeValueSetName": "Test"}} + assert call_args.kwargs["headers"] is headers + out = capsys.readouterr().out + assert "Test" in out + assert "vl-id" in out + + +def test_activate_variable_library_value_set_failure_exits() -> None: + fake_resp = mock.Mock(status_code=400, text="Bad value set name") + headers = {"Authorization": "Bearer x"} + with mock.patch("deploy_bulk.requests.patch", return_value=fake_resp): + from deploy_bulk import activate_variable_library_value_set + with pytest.raises(SystemExit) as exc: + activate_variable_library_value_set( + workspace_id="ws", library_id="vl", + value_set_name="Bad", headers=headers, + ) + assert "Failed to set active value set" in str(exc.value) + assert "400" in str(exc.value) diff --git a/tests/test_workspace_swap.py b/tests/test_workspace_swap.py index 96f6999..209b2ce 100644 --- a/tests/test_workspace_swap.py +++ b/tests/test_workspace_swap.py @@ -436,40 +436,17 @@ def test_windows_line_endings(self, tmp_path: Path): @pytest.mark.usefixtures("_patch_paths") class TestResolveFeatureIds: - """Priority order: existing value set -> .env -> interactive prompt.""" + """Source of truth: .env at repo root. Missing/blank → exit with error.""" def _vs_path(self, fabric_dir: Path) -> Path: return fabric_dir / "Patterns_Variables.VariableLibrary" / "valueSets" / "feat.json" - def _write_value_set(self, path: Path, ws_id: str, lh_id: str) -> None: - path.write_text(json.dumps({ - "name": "feat", - "variableOverrides": [ - {"name": "target_workspace_id", "value": ws_id}, - {"name": "target_lakehouse_id", "value": lh_id}, - ], - }), encoding="utf-8") - def _write_env(self, fabric_dir: Path, content: str) -> None: # ENV_FILE = REPO_ROOT / ".env"; in the test fixture REPO_ROOT == tmp_path env_path = fabric_dir.parent.parent / ".env" env_path.write_text(content, encoding="utf-8") - def test_value_set_takes_priority(self, fabric_dir: Path): - from workspace_swap import resolve_feature_ids - vs_path = self._vs_path(fabric_dir) - self._write_value_set(vs_path, FEAT_WS_ID, FEAT_LH_ID) - # Even with an .env present pointing elsewhere, value set wins. - self._write_env( - fabric_dir, - f"FEATURE_WORKSPACE_ID=99999999-9999-9999-9999-999999999999\n" - f"FEATURE_LAKEHOUSE_ID=88888888-8888-8888-8888-888888888888\n", - ) - ws_id, lh_id = resolve_feature_ids("feat", vs_path) - assert ws_id == FEAT_WS_ID - assert lh_id == FEAT_LH_ID - - def test_env_file_used_when_no_value_set(self, fabric_dir: Path): + def test_env_file_resolves_ids(self, fabric_dir: Path): from workspace_swap import resolve_feature_ids vs_path = self._vs_path(fabric_dir) self._write_env( @@ -480,112 +457,198 @@ def test_env_file_used_when_no_value_set(self, fabric_dir: Path): assert ws_id == FEAT_WS_ID assert lh_id == FEAT_LH_ID - def test_value_set_with_partial_data_falls_through_to_env(self, fabric_dir: Path): + def test_env_takes_precedence_over_existing_value_set(self, fabric_dir: Path): + """A pre-existing value set must NOT override .env. Stale value sets + were the cause of a wrong-workspace swap before this change.""" from workspace_swap import resolve_feature_ids vs_path = self._vs_path(fabric_dir) - # Only target_workspace_id, no target_lakehouse_id + vs_path.parent.mkdir(parents=True, exist_ok=True) vs_path.write_text(json.dumps({ "name": "feat", "variableOverrides": [ - {"name": "target_workspace_id", "value": FEAT_WS_ID}, + {"name": "target_workspace_id", "value": "99999999-9999-9999-9999-999999999999"}, + {"name": "target_lakehouse_id", "value": "88888888-8888-8888-8888-888888888888"}, ], }), encoding="utf-8") self._write_env( fabric_dir, - f"FEATURE_WORKSPACE_ID=99999999-9999-9999-9999-999999999999\n" - f"FEATURE_LAKEHOUSE_ID={FEAT_LH_ID}\n", + f"FEATURE_WORKSPACE_ID={FEAT_WS_ID}\nFEATURE_LAKEHOUSE_ID={FEAT_LH_ID}\n", ) ws_id, lh_id = resolve_feature_ids("feat", vs_path) - # Falls through to .env, so .env values are returned (not partial value set) - assert ws_id == "99999999-9999-9999-9999-999999999999" + assert ws_id == FEAT_WS_ID assert lh_id == FEAT_LH_ID - def test_no_value_set_no_env_prompts(self, fabric_dir: Path): + def test_no_env_file_exits(self, fabric_dir: Path): from workspace_swap import resolve_feature_ids vs_path = self._vs_path(fabric_dir) - with patch("builtins.input", side_effect=[FEAT_WS_ID, FEAT_LH_ID]): - ws_id, lh_id = resolve_feature_ids("feat", vs_path) - assert ws_id == FEAT_WS_ID - assert lh_id == FEAT_LH_ID + # No .env created. + with pytest.raises(SystemExit) as exc: + resolve_feature_ids("feat", vs_path) + assert ".env" in str(exc.value) - def test_env_missing_workspace_key_falls_through_to_prompt(self, fabric_dir: Path): + def test_env_missing_workspace_key_exits(self, fabric_dir: Path): from workspace_swap import resolve_feature_ids vs_path = self._vs_path(fabric_dir) self._write_env(fabric_dir, f"FEATURE_LAKEHOUSE_ID={FEAT_LH_ID}\n") - with patch("builtins.input", side_effect=[FEAT_WS_ID, FEAT_LH_ID]): - ws_id, lh_id = resolve_feature_ids("feat", vs_path) - assert ws_id == FEAT_WS_ID - assert lh_id == FEAT_LH_ID + with pytest.raises(SystemExit) as exc: + resolve_feature_ids("feat", vs_path) + assert "FEATURE_WORKSPACE_ID" in str(exc.value) - def test_env_missing_lakehouse_key_falls_through_to_prompt(self, fabric_dir: Path): + def test_env_missing_lakehouse_key_exits(self, fabric_dir: Path): from workspace_swap import resolve_feature_ids vs_path = self._vs_path(fabric_dir) self._write_env(fabric_dir, f"FEATURE_WORKSPACE_ID={FEAT_WS_ID}\n") - with patch("builtins.input", side_effect=[FEAT_WS_ID, FEAT_LH_ID]): - ws_id, lh_id = resolve_feature_ids("feat", vs_path) - assert ws_id == FEAT_WS_ID - assert lh_id == FEAT_LH_ID - - def test_env_invalid_guid_exits(self, fabric_dir: Path): - from workspace_swap import resolve_feature_ids - vs_path = self._vs_path(fabric_dir) - self._write_env( - fabric_dir, - "FEATURE_WORKSPACE_ID=not-a-guid\n" - f"FEATURE_LAKEHOUSE_ID={FEAT_LH_ID}\n", - ) - with pytest.raises(SystemExit): + with pytest.raises(SystemExit) as exc: resolve_feature_ids("feat", vs_path) + assert "FEATURE_LAKEHOUSE_ID" in str(exc.value) - def test_env_empty_workspace_value_falls_through_to_prompt(self, fabric_dir: Path): + def test_env_blank_workspace_value_exits(self, fabric_dir: Path): from workspace_swap import resolve_feature_ids vs_path = self._vs_path(fabric_dir) self._write_env( fabric_dir, f"FEATURE_WORKSPACE_ID=\nFEATURE_LAKEHOUSE_ID={FEAT_LH_ID}\n", ) - with patch("builtins.input", side_effect=[FEAT_WS_ID, FEAT_LH_ID]): - ws_id, lh_id = resolve_feature_ids("feat", vs_path) - assert ws_id == FEAT_WS_ID - assert lh_id == FEAT_LH_ID + with pytest.raises(SystemExit) as exc: + resolve_feature_ids("feat", vs_path) + assert "FEATURE_WORKSPACE_ID" in str(exc.value) - def test_env_empty_lakehouse_value_falls_through_to_prompt(self, fabric_dir: Path): + def test_env_blank_lakehouse_value_exits(self, fabric_dir: Path): from workspace_swap import resolve_feature_ids vs_path = self._vs_path(fabric_dir) self._write_env( fabric_dir, f"FEATURE_WORKSPACE_ID={FEAT_WS_ID}\nFEATURE_LAKEHOUSE_ID=\n", ) - with patch("builtins.input", side_effect=[FEAT_WS_ID, FEAT_LH_ID]): - ws_id, lh_id = resolve_feature_ids("feat", vs_path) - assert ws_id == FEAT_WS_ID - assert lh_id == FEAT_LH_ID + with pytest.raises(SystemExit) as exc: + resolve_feature_ids("feat", vs_path) + assert "FEATURE_LAKEHOUSE_ID" in str(exc.value) - def test_env_both_empty_values_falls_through_to_prompt(self, fabric_dir: Path): + def test_env_whitespace_only_value_exits(self, fabric_dir: Path): from workspace_swap import resolve_feature_ids vs_path = self._vs_path(fabric_dir) self._write_env( fabric_dir, - "FEATURE_WORKSPACE_ID=\nFEATURE_LAKEHOUSE_ID=\n", + 'FEATURE_WORKSPACE_ID=" "\n' + f'FEATURE_LAKEHOUSE_ID="{FEAT_LH_ID}"\n', ) - with patch("builtins.input", side_effect=[FEAT_WS_ID, FEAT_LH_ID]): - ws_id, lh_id = resolve_feature_ids("feat", vs_path) - assert ws_id == FEAT_WS_ID - assert lh_id == FEAT_LH_ID + with pytest.raises(SystemExit) as exc: + resolve_feature_ids("feat", vs_path) + assert "FEATURE_WORKSPACE_ID" in str(exc.value) - def test_env_whitespace_only_value_falls_through_to_prompt(self, fabric_dir: Path): + def test_env_invalid_guid_exits(self, fabric_dir: Path): from workspace_swap import resolve_feature_ids vs_path = self._vs_path(fabric_dir) self._write_env( fabric_dir, - 'FEATURE_WORKSPACE_ID=" "\n' - f'FEATURE_LAKEHOUSE_ID="{FEAT_LH_ID}"\n', + "FEATURE_WORKSPACE_ID=not-a-guid\n" + f"FEATURE_LAKEHOUSE_ID={FEAT_LH_ID}\n", ) - with patch("builtins.input", side_effect=[FEAT_WS_ID, FEAT_LH_ID]): - ws_id, lh_id = resolve_feature_ids("feat", vs_path) + with pytest.raises(SystemExit): + resolve_feature_ids("feat", vs_path) + + +# ── _confirm_swap_to_feature ────────────────────────────────────────────── + +class TestConfirmSwapToFeature: + """Confirmation gating before any files are rewritten by swap-to-feature.""" + + def test_yes_uppercase_proceeds(self): + from workspace_swap import _confirm_swap_to_feature + with patch("builtins.input", return_value="YES"): + # Should return without raising. + _confirm_swap_to_feature("feat", DEV_WS_ID, DEV_LH_ID, FEAT_WS_ID, FEAT_LH_ID) + + def test_yes_lowercase_aborts(self): + from workspace_swap import _confirm_swap_to_feature + with patch("builtins.input", return_value="yes"): + with pytest.raises(SystemExit) as exc: + _confirm_swap_to_feature("feat", DEV_WS_ID, DEV_LH_ID, FEAT_WS_ID, FEAT_LH_ID) + assert "Aborted" in str(exc.value) + + def test_y_aborts(self): + from workspace_swap import _confirm_swap_to_feature + with patch("builtins.input", return_value="y"): + with pytest.raises(SystemExit) as exc: + _confirm_swap_to_feature("feat", DEV_WS_ID, DEV_LH_ID, FEAT_WS_ID, FEAT_LH_ID) + assert "Aborted" in str(exc.value) + + def test_blank_aborts(self): + from workspace_swap import _confirm_swap_to_feature + with patch("builtins.input", return_value=""): + with pytest.raises(SystemExit) as exc: + _confirm_swap_to_feature("feat", DEV_WS_ID, DEV_LH_ID, FEAT_WS_ID, FEAT_LH_ID) + assert "Aborted" in str(exc.value) + + def test_eof_aborts(self): + from workspace_swap import _confirm_swap_to_feature + with patch("builtins.input", side_effect=EOFError): + with pytest.raises(SystemExit) as exc: + _confirm_swap_to_feature("feat", DEV_WS_ID, DEV_LH_ID, FEAT_WS_ID, FEAT_LH_ID) + assert "Aborted" in str(exc.value) + + def test_yes_with_surrounding_whitespace_proceeds(self): + """input().strip() removes surrounding whitespace; case still matters.""" + from workspace_swap import _confirm_swap_to_feature + with patch("builtins.input", return_value=" YES "): + _confirm_swap_to_feature("feat", DEV_WS_ID, DEV_LH_ID, FEAT_WS_ID, FEAT_LH_ID) + + def test_planned_swap_is_displayed(self, capsys: pytest.CaptureFixture): + """User must see the planned IDs before being asked to confirm.""" + from workspace_swap import _confirm_swap_to_feature + with patch("builtins.input", return_value="YES"): + _confirm_swap_to_feature("my-branch", DEV_WS_ID, DEV_LH_ID, FEAT_WS_ID, FEAT_LH_ID) + out = capsys.readouterr().out + assert "my-branch" in out + assert DEV_WS_ID in out + assert FEAT_WS_ID in out + assert DEV_LH_ID in out + assert FEAT_LH_ID in out + + +# ── _read_previous_feature_ids ──────────────────────────────────────────── + +class TestReadPreviousFeatureIds: + """Recovery support: read the IDs a previous swap applied so the next + swap can rewrite them if .env has changed.""" + + def test_returns_none_when_value_set_missing(self, tmp_path: Path): + from workspace_swap import _read_previous_feature_ids + ws_id, lh_id = _read_previous_feature_ids(tmp_path / "missing.json") + assert ws_id is None + assert lh_id is None + + def test_reads_both_ids_from_value_set(self, tmp_path: Path): + from workspace_swap import _read_previous_feature_ids + path = tmp_path / "feat.json" + path.write_text(json.dumps({ + "name": "feat", + "variableOverrides": [ + {"name": "target_workspace_id", "value": FEAT_WS_ID}, + {"name": "target_lakehouse_id", "value": FEAT_LH_ID}, + ], + }), encoding="utf-8") + ws_id, lh_id = _read_previous_feature_ids(path) assert ws_id == FEAT_WS_ID assert lh_id == FEAT_LH_ID + def test_returns_none_for_missing_overrides(self, tmp_path: Path): + from workspace_swap import _read_previous_feature_ids + path = tmp_path / "feat.json" + path.write_text(json.dumps({"name": "feat", "variableOverrides": []}), encoding="utf-8") + ws_id, lh_id = _read_previous_feature_ids(path) + assert ws_id is None + assert lh_id is None + + def test_returns_none_when_value_set_malformed(self, tmp_path: Path): + """A corrupt or non-JSON value-set file shouldn't crash the swap.""" + from workspace_swap import _read_previous_feature_ids + path = tmp_path / "feat.json" + path.write_text("not valid json", encoding="utf-8") + ws_id, lh_id = _read_previous_feature_ids(path) + assert ws_id is None + assert lh_id is None + # ── _validate_guid ──────────────────────────────────────────────────────── @@ -660,7 +723,8 @@ def test_dev_branch_exits(self, monkeypatch): def test_feature_branch_does_not_exit_early(self, monkeypatch, fabric_dir: Path): """Branch guard must not block feature branches. We let main() proceed as far as resolve_feature_ids, where we feed it a valid .env so the - full flow completes without prompting.""" + full flow completes. The confirmation prompt is auto-answered with + YES.""" import workspace_swap env_path = fabric_dir.parent.parent / ".env" env_path.write_text( @@ -670,7 +734,8 @@ def test_feature_branch_does_not_exit_early(self, monkeypatch, fabric_dir: Path) monkeypatch.setattr(sys, "argv", ["workspace_swap.py"]) monkeypatch.setattr(workspace_swap, "get_current_branch", lambda: "feature/safe") # No SystemExit expected - workspace_swap.main() + with patch("builtins.input", return_value="YES"): + workspace_swap.main() # Side effect: value set was created vs_path = ( fabric_dir / "Patterns_Variables.VariableLibrary" @@ -700,7 +765,8 @@ def test_env_with_feature_ids_creates_value_set_and_repoints(self, fabric_dir: P from workspace_swap import _run_swap_to_feature, load_json, SETTINGS_FILE self._write_env(fabric_dir, FEAT_WS_ID, FEAT_LH_ID) vs_path = self._vs_path(fabric_dir, "feat") - _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) + with patch("builtins.input", return_value="YES"): + _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) # Value set written with correct overrides assert vs_path.exists() @@ -736,7 +802,8 @@ def test_partial_dev_id_match_does_not_block(self, fabric_dir: Path): # Same workspace ID, different lakehouse self._write_env(fabric_dir, DEV_WS_ID, FEAT_LH_ID) vs_path = self._vs_path(fabric_dir, "feat") - _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) + with patch("builtins.input", return_value="YES"): + _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) assert vs_path.exists() def test_dry_run_makes_no_changes(self, fabric_dir: Path): @@ -753,20 +820,83 @@ def test_dry_run_makes_no_changes(self, fabric_dir: Path): assert sm_file.read_text(encoding="utf-8") == original_sm assert load_json(SETTINGS_FILE) == original_settings + def test_dry_run_skips_confirmation(self, fabric_dir: Path): + """--dry-run is the verification step; no confirmation prompt.""" + from workspace_swap import _run_swap_to_feature + self._write_env(fabric_dir, FEAT_WS_ID, FEAT_LH_ID) + vs_path = self._vs_path(fabric_dir, "feat") + # If the prompt fired, input() would be called and raise OSError + # under pytest's stdin capture. The fact that this passes without + # patching input() proves dry-run skips confirmation. + _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=True) + def test_idempotent_swap_to_feature(self, fabric_dir: Path): """Running swap-to-feature twice on the same branch must not error or duplicate state.""" from workspace_swap import _run_swap_to_feature, load_json, SETTINGS_FILE self._write_env(fabric_dir, FEAT_WS_ID, FEAT_LH_ID) vs_path = self._vs_path(fabric_dir, "feat") - _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) - # Second run: value set already exists, items already repointed - _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) + with patch("builtins.input", return_value="YES"): + _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) + # Second run: value set already exists, items already repointed + _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) assert vs_path.exists() settings = load_json(SETTINGS_FILE) assert settings["valueSetsOrder"].count("feat") == 1 + def test_recovers_from_stale_feature_ids_in_files(self, fabric_dir: Path): + """Regression for the wrong-.env scenario: a previous swap applied + stale feature IDs; .env now points at the correct workspace; re-running + the swap must rewrite the stale IDs to the correct ones (recovery + pass), without needing manual cleanup of the SemanticModel/Notebook + files.""" + from workspace_swap import _run_swap_to_feature + STALE_WS_ID = "99999999-9999-9999-9999-999999999999" + STALE_LH_ID = "88888888-8888-8888-8888-888888888888" + + # First: simulate a prior bad swap by writing the stale IDs into the + # value set and the SemanticModel file. + self._write_env(fabric_dir, STALE_WS_ID, STALE_LH_ID) + vs_path = self._vs_path(fabric_dir, "feat") + with patch("builtins.input", return_value="YES"): + _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) + + sm_file = fabric_dir / "Test.SemanticModel" / "definition" / "expressions.tmdl" + assert STALE_WS_ID in sm_file.read_text(encoding="utf-8") + + # Now: correct .env and re-run. Recovery pass should rewrite the + # stale IDs even though dev IDs are no longer in the file. + self._write_env(fabric_dir, FEAT_WS_ID, FEAT_LH_ID) + with patch("builtins.input", return_value="YES"): + _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) + + content = sm_file.read_text(encoding="utf-8") + assert FEAT_WS_ID in content + assert FEAT_LH_ID in content + assert STALE_WS_ID not in content + assert STALE_LH_ID not in content + + def test_no_recovery_pass_when_stale_matches_target(self, fabric_dir: Path): + """When the value set already has the target IDs (e.g., a prior run + succeeded), the recovery pass must not run \u2014 nothing to recover.""" + from workspace_swap import _run_swap_to_feature + self._write_env(fabric_dir, FEAT_WS_ID, FEAT_LH_ID) + vs_path = self._vs_path(fabric_dir, "feat") + + # First run: applies feature IDs cleanly. + with patch("builtins.input", return_value="YES"): + _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) + + # Second run with the same .env: capture stdout to confirm no recovery + # log line. + with patch("builtins.input", return_value="YES"): + with patch("builtins.print") as mock_print: + _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) + + all_output = " ".join(str(call) for call in mock_print.call_args_list) + assert "Recovery:" not in all_output + # ── _run_swap_to_dev orchestration ───────────────────────────────────────────── @@ -789,7 +919,9 @@ def test_round_trip_swap_to_feature_then_dev(self, fabric_dir: Path): sm_file = fabric_dir / "Test.SemanticModel" / "definition" / "expressions.tmdl" original_sm = sm_file.read_text(encoding="utf-8") - _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) + with patch("builtins.input", return_value="YES"): + _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) + # swap-to-dev does not prompt for confirmation. _run_swap_to_dev("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) # Items reverted to dev IDs @@ -821,7 +953,8 @@ def test_swap_to_dev_does_not_read_env(self, fabric_dir: Path): vs_path = self._vs_path(fabric_dir, "feat") sm_file = fabric_dir / "Test.SemanticModel" / "definition" / "expressions.tmdl" original_sm = sm_file.read_text(encoding="utf-8") - _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) + with patch("builtins.input", return_value="YES"): + _run_swap_to_feature("feat", "feat", vs_path, DEV_WS_ID, DEV_LH_ID, dry_run=False) # Now corrupt the .env (different IDs from what's actually in the items) env_path.write_text(