-
Notifications
You must be signed in to change notification settings - Fork 730
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] Extendable Param Files
feature
#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)! |
Extendable Param Files
feature
I think this new design change i a breakaway from the traditional way of thinking about defining parameters, and i think it will be worse because of it. I am a huge fan of any input being explicit for any deployment, so when you look at a bicep + pram file you can se what actual input parameter is being used and what is still missing.. or optionals being set/not set. b: value set in a 'shared' param file will be set at deploytime, even if you didn't want to set it. from my experience you should be able to extend the definition of a thing, but not the invocation/implementation (class can be expanded using inheritance or extended in js/typescript, but once you call that class you have to provide all required arguments) now i'm not against having a shared param structure that can set commonly defined values, however this should be a "opt in", like you already do in "import": // base.bicepparam
using none
param foo = 'foo'
param bar = 'bar'
// main.bicepparam
using 'main.bicep'
import * as shared from 'base.bicepparam'
param foo = 'bar'
param baz = foo
param bar = shared.bar
param other = shared.foo
// compiled.json
{
foo: 'bar'
bar: 'bar'
baz: 'bar'
other:'foo'
} this way it is no question as to what param is being used where. now if the reason is to provide different arguments depending on the environment, why not mplement env var checks? setting a |
@WithHolm isn't the capability you're asking for already supported today?
To answer some of the concerns you raised:
True, but it's a choice - only if the user wants to structure their code this way. IMO the following semantics are important to avoid surprise:
Given the above semantics, I can only see this causing a problem in the case where the user wants to rely on the parameter default value in the In this example, the behavior doesn't feel particularly surprising to me. If the user has opted in to using default values, they have already opted to move away from explicitly setting parameter values in their |
My feedback from the pull request: I find the proposed design very limited. As far as I understood from the meeting the following things are not possible:
I think better approach would be to have something like:
In this scenario you are clearly defining that this is shared bicep parameters file. Also additionally you provide two (or more if you want, these are optional statements) bicep files to serve as intelisense for the shared file. In this scenario when you define that this is shared bicep parameters files any errors related to required parameters are turned off. In VSC you can still see from intelisense if specific parameter is required or not. You can also see the description for those parameters. In case main1.bicep and main2.bicep contains the same parameter let's say location and that contains two different descriptions upon hovering the parameter when added you can see the description for both. In summary you still get intelisense and other information like description, allowed values, etc, but without the errors you would get in regular bicep parameters files. With that said I would like to see support for the rest of the missing features as well:
Would also be nice that if you can declare which parameters from extends statement you would like to add instead of adding everything. Similar to how it is in import statements. It would really make sense if all these features are released at once otherwise it is best to be preview feature until every feature is added. |
My feedback after watching the latest demo: I have watched the last meeting but did not see any of the feedback given before implemented. In fact it seems that everything was the same and the only small change is that using is having none for value instead of null. That is only cosmetic change and it is irrelevant for the functionality. Kind of disappointed due to that. |
@slavizh - thank you for sharing your thoughts! @stephaniezyen is going to work on organizing all feedback we're receiving on this new capability, and use this to prioritize further changes. |
@slavizh - Hi Stan, I've opened issues for each of the features you listed - like Anthony said, we're planning on putting together feedback on improvements we want to make for Extendable Param Files as well as .bicepparam files as a whole. I failed to mention in the community call, so that's my mistake, but this is an experimental feature for now! We want to receive all feedback on usage and any improvements we may want to make before making it a public feature like you mentioned. |
@stephaniezyen Thanks Stephanie! |
Extendable Param Files
Feature Design ProposalAs a
Bicep developer
, I want to be able toextend bicepparam files
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 reuse it in other.bicepparam
files.Goal is to be able to extend parameter files from others with an extends directive
Proposed Solution
Syntax
Scenario
Combine parameters from the
main.bicepparam
, and a separate.bicepparam
file;extends
statements can only be used in.bicepparam
files.using
statements are required in.bicepparam
files, so, in shared/base.bicepparam
files, theusing none
statement is required.The text was updated successfully, but these errors were encountered: