Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow dependsOn for existing resources #15459

Merged
merged 1 commit into from
Nov 1, 2024
Merged

Conversation

jeskew
Copy link
Contributor

@jeskew jeskew commented Nov 1, 2024

Resolves #13596

Microsoft Reviewers: Open in CodeFlow

Copy link
Contributor

github-actions bot commented Nov 1, 2024

Test this change out locally with the following install scripts (Action run 11633757585)

VSCode
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-vsix.sh) --run-id 11633757585
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-vsix.ps1) } -RunId 11633757585"
Azure CLI
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-cli.sh) --run-id 11633757585
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-cli.ps1) } -RunId 11633757585"

Copy link
Contributor

github-actions bot commented Nov 1, 2024

Dotnet Test Results

    72 files   -     36      72 suites   - 36   28m 59s ⏱️ - 12m 39s
11 379 tests  -     23  11 379 ✅  -     23  0 💤 ±0  0 ❌ ±0 
26 489 runs   - 13 234  26 489 ✅  - 13 234  0 💤 ±0  0 ❌ ±0 

Results for commit b435471. ± Comparison against base commit 6bf7499.

@levimatheri
Copy link
Contributor

levimatheri commented Nov 1, 2024

the presence of any existing resources with explicit dependencies should result in Bicep using symbolic name compilation

Going by the above comment on the linked issue. I was curious: where are we detecting the dependsOn on existing in order to "auto-upgrade" to use symbolic names? Not seeing that in this PR...

@jeskew
Copy link
Contributor Author

jeskew commented Nov 1, 2024

the presence of any existing resources with explicit dependencies should result in Bicep using symbolic name compilation

Going by the above comment on the linked issue. I was curious: where are we detecting the dependsOn on existing in order to "auto-upgrade" to use symbolic names? Not seeing that in this PR...

That change ended up being unnecessary. The dependency inference code in Bicep will take a dependency on an existing resource's transitive dependencies, so the dependsOn ends up getting respected.

@levimatheri levimatheri requested review from levimatheri and removed request for levimatheri November 1, 2024 17:39
@jeskew jeskew merged commit 74733c6 into main Nov 1, 2024
47 checks passed
@jeskew jeskew deleted the jeskew/existing-dependsOn branch November 1, 2024 17:40
jeskew added a commit that referenced this pull request Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow dependsOn on existing resources
3 participants