persist os env sub and location to env file during ensureEnv #3049
Conversation
vhvb1989
requested review from
ellismg,
wbreza,
hemarina and
weikanglim
as code owners
|
@timheuer , this change should fix the LOCATION issue. Once there is a installable version from this PR, you can add it to your dev branch and that should get your gh-action running end2end |
|
Generally, is there a reason these are not first checked to see if there is an actual environment variable already there before checking from a .env specifically? |
|
@timheuer , both |
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSIContainerDocumentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
|
Related to #2423? |
|
Some minor feedback: I wonder if someone who thinks about azd
Expected: Temporarily provision in YMMV, I proposed a change previously in #2426 to provide this behavior. The feedback I received there could be solved if the "subscription prompter" and "location prompter" all hinge on the same env key and it's clear that the prompts would no-opt with default in case where the variable is set via |
@weikanglim , while this approach seems tempting, I would promote it as an anti-pattern. The azd-env is specifically meant to support that case in a more general approach, where you can do Thinking about overriding one only var within azd-env is dangerous, as the temporal outcome is not backed up with any azd-env. So it would be like a cloud-leak (as a ref to mem-leak) |
|
@vhvb1989 I'm trying to advocate for the user's expectation here (regardless of what is "best practice" -- which can be subjective) is that azd works similarly with However, if we believe the slightly heavy-handed approach to always introduce sync writes to force the system to be in a certain state is the desirable and cleaner outcome, I'm not going to push any further, and happy with what we have. |
I see it ideal, more than desirable, lol. There's currently a dependency from some commands like |
Wouldn't the commands query both .env and system env, like how the
Same thing here. Why do we decide in some cases to not look at both |
|
I did look into it a little, and there's definitely more consideration to be had if we wanted to expose |
fix: #3031
This change updates
EnsureSubscriptionAndLocation()to make sure the subscription and location are written to the .env when the values are read from the system environment.Explanation:
During
provision, subscription and location can be read from system environment and provided in-memory to bicep or terraform provider to deploy resources.For operations coming after
provision, the expectation is to find the required values to continue from .env, and not to query the system env any more. Some operation might even pull the env.values as a map and use it, instead of using the env object to pull values (which would fall back to system env when value is not found).For the issue on #3031, even though AZURE_LOCATION is set as env-var, when
azd provisionruns, the location is not persisted on .env file. Later, a follow up operation is trying to pull the location from .env.