-
Notifications
You must be signed in to change notification settings - Fork 726
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
[Design Proposal] Modular Parameters #12019
Comments
Will there by a specific syntax for importing all param values from a file? |
I like these ideas
The env variable idea could be done by extending the current var usage to the above
I'm not sure the named import would be needed with the above approach as any imported param should just be able to be referenced from the Alternatively perhaps an |
will this support for exmaple default.bicep
eastus.bicep
eastus.json would be
|
I would like to see both a wildcard import to import all parameters from a given file in addition to selecting specific params. How would name conflicts be handled between files? Personally, I'd prefer a preference towards specificity (e.g. import the specifically named import over a wildcard, but show a squiggly line and build warning indicating the conflict). |
Expanding on this, seen customers where they like the generate a parameter file for an entire environment which will support multiple deployments. Was wondering if we could then do selective filtering based on an initial import of parameters. Rough code example of this idea
|
great question! Importing everything from a separate file could be dangerous, since you have to provide only the parameters required in the deployment, and that separate file could be updated and have new parameters for totally different deployments. This could make the deployments that have "everything" syntax fail. We'd like to have a conservative approach here, to make sure if the deployment can be made with the current content of the files, it'll still be made even if you add new parameters into the other file. But we're thinking to add some features into the VSCode Extension, so authors of bicepparam files will easily get the list of parameters that can be imported. |
in different places in bicep, we already have |
we intentionally won't support everything syntax, the main reason is, I also mentioned this issue here; #12019 (comment) |
Wildcard imports could be dangerous and can make the deployments that have no change, fail. (as mentioned here; #12019 (comment)) I'd like to understand better, what's the use-case of a wildcard import, if there is an editor support and you can easily import required parameters? I see wildcard import syntax is to make importing several parameters easily. is it still the case, if the editor helps you to import required parameters easily? |
bicep (vscode and vs extensions, language server, cli, etc.) parse bicep and bicepparam files and we're not going to support wildcard import syntax. so, developers should import parameters by their names, and if there is a conflict, developer should resolve the conflict.
|
I think it's a very interesting idea. Although how much I like the idea, I don't see better developer experience between these two code blocks; allParams = import { * } from './global/${readEnvironmentVariable('env')}.bicepparam
import { location, name } from allParams import { location, name } from './global/${readEnvironmentVariable('env')}.bicepparam in fact, the first proposal seems more complex, and, longer to type 😄 two |
More to the point, developers have been spoiled and are fairly used to having the option of wildcard imports. They're extremely common in other languages. I've got to imagine there'd be a sizable number of issues over time of new Bicep developers inquiring as to what they're doing wrong in that their wildcard import statements don't work for parameters, but they do for user-defined types.
Even with some editor support, it's a poor experience in Resharper already when I drop in a wall of new types into a class and inevitably have to go one by one through the new types to have it locate and add the correct using statement for each. Given the opportunity, I'd rather avoid a repeat of that poor UX here as well.
I've been an advocate for constant-folding + reliable what-if local simulated deployments and this just feels like another excellent use case. Even today absent parameter files, a change to a parameter can break a deployment and there's no way to know about it absent repeated real deployments to Azure in temporary resource groups (which costs real money). I personally wouldn't rely on re-using parameter files in the short term because the tooling simply isn't here to fully validate complex deployments with anywhere near the accuracy of a real deployment. I mean, to your point, the parameters you started with in eastus.bicepparam could easily change over time simply because someone forgot they were already used elsewhere... but wildcard or specific import statements don't help there either. Your objection is little different from user types - those might change over time and have non-default parameters added which would break existing uses of them.. and as I mentioned above, they support wildcard imports. |
That seems more opinionated than conservative. Why go out of the way to deliberately take away the choice from developers and make the language less consistent? Without wildcard, adding a shared parameter results in modifying all parameter files that use it. Whether I have editor support or not, every file needs to be modified, every file is changed in git, and every file needs to be reviewed. And probably doing the change without editor help will be quicker. |
Just catching up on the community calls. Are you still looking for feedback? Is this work already underway? |
Would love to see this feature. There is so much copy and paste params across my |
Is this far from finishing? This would finally let me move away from having to use CUE lang for parameters... |
I believe we are getting close to getting this done and released as an experimental feature. @stephaniezyen / @polatengin can provide a more precise ETA. |
we have a draft PR is up (#13900) our plan is to make it to the next release, behind a feature flag. |
Some comments here: #13900 (comment) |
Are we any closer to this being merged in yet? |
@ryancolley Unfortunately, we've had a longer delay than anticipated due to design decisions and changes. However, we're hoping to have it out as an experimental feature by the end of this month (hopefully)! |
Modular Parameters Design Proposal
As a
Bicep developer
, I want to be able toimport parameters
fromother bicepparam files
, so that I canreuse
parameters acrossmultiple deployments
.In most deployment scenarios, there are parameters that are common across multiple deployments. For example, a
location
parameter is often used in multiple deployments. It would be useful to be able to define thelocation
parameter in a single file, and import it into other .bicepparam files.Also it would be useful to be able to define parameters in
environment
specific files, and import them into themain.bicepparam
file. For example, alocation
parameter can be defined indev.bicepparam
andprod.bicepparam
files, and themain.bicepparam
file can import thelocation
parameter from thedev.bicepparam
file when deploying to thedev
environment, and import thelocation
parameter from theprod.bicepparam
file when deploying to theprod
environment.Goals
Parent GitHub Issue: Bicep Parameters Upcoming Features #10777
Design Considerations
.bicepparam
files (more than one import is possible)readEnvironmentVariable()
functionProposed Solution
Syntax
Scenario 1
Combine parameters from the
main.bicepparam
, and two different.bicepparam
files;Scenario 2
main.bicepparam
file imports parameters and alias one of the parameters, so it'll berenamed
.Scenario 3
Import parameters from a
.bicepparam
file, resolving the path of the.bicepparam
usingreadEnvironmentVariable()
functionScenario 4
Import parameters from multiuple
.bicepparam
file, resolving the path of the.bicepparam
usingreadEnvironmentVariable()
functionThe text was updated successfully, but these errors were encountered: