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

Add Optional KeyName Param to Items() Function #9503

Closed
RiverHeart opened this issue Jan 9, 2023 · 3 comments
Closed

Add Optional KeyName Param to Items() Function #9503

RiverHeart opened this issue Jan 9, 2023 · 3 comments
Labels
enhancement New feature or request Needs: Upvote This issue requires more votes to be considered

Comments

@RiverHeart
Copy link

RiverHeart commented Jan 9, 2023

As mentioned in #1427, I wish you could pass in a KeyName param to the items() function like items(myObj, "name"). This change would make it easier for users to work with Bicep properties that only accept NameValuePairs and not KeyValuePairs. One such property is SiteConfig/Appsettings and although I can use map((items(appsettings)), arg => {name: arg.key, value: arg.value}) to reconstruct the object list it feels inelegant.

I guess another way of going about this would be making NameValuePair and KeyValuePair interchangeable data types but I imagine those changes would be more work than my proposal.


Also, I realize this is a pipe dream but the NameValuePair stuff makes SiteConfig/Appsettings difficult to work with because you can't do StructuredTransforms on a parameter file when the key names are all the same. Additionally, the items() function only works on the first depth of an object and there is no way to recurse a child object (far as I know) to produce a flattened object. So, a single level appsettings.json could work but users will probably make the mistake of thinking they can nest and we can't validate a JSON schema against it yet.

If we had a flatten() function which could take

{
    "Foo": {
        "Bar1": "Val1",
        "Bar2": "Val2"
    }   
}

and convert it to

{
    "Foo--Bar1": "Val1",
    "Foo--Bar2": "Val2"   
}

that would allow us to use items() to convert it to NameValuePair and pass it to SiteConfig/Appsettings with minimal (user) effort. I imagine that it would look like this items(flatten(loadJsonContent('appsettings.json'), "--"), "name") in practice. Maybe that's where User Defined Functions will come in down the road? I guess I can't really think of where else a flatten() function would come in handy so... yeah.

Anyway, thanks for listening.

@RiverHeart RiverHeart added the enhancement New feature or request label Jan 9, 2023
@ghost ghost added the Needs: Triage 🔍 label Jan 9, 2023
@stephaniezyen stephaniezyen added Needs: Upvote This issue requires more votes to be considered and removed Needs: Triage 🔍 labels Jan 11, 2023
@github-project-automation github-project-automation bot moved this to Todo in Bicep Jan 11, 2023
@anthony-c-martin
Copy link
Member

Out of interest, why do you feel like the map approach you suggested is inelegant? That would be my preferred approach to accomplish this scenario:

map((items(appsettings)), arg => {name: arg.key, value: arg.value})

@RiverHeart
Copy link
Author

RiverHeart commented Jan 11, 2023

Well, it's not terrible but NVP is a standard datatype so it felt like there ought to be a shorter, normal'ish looking method like passing in a param or casting the KVP to an NVP to get the right value. It took me longer than it should have to realize the only difference between the working and non-working object was just the key name. The appsettings property doesn't complain about being given an invalid object, it just fails.

If the name is seeded from the start the intent is clear that we want the format returned by items() just with a different name. Then again, if map() treats all objects as KVPs (I'm new to this btw) maybe I could have done a map(appsettings), arg => {name: arg.key, value: arg.value}) which would have been a little better but I've reinvented items() or names() rather :)

I get it though, it's nothing egregious.

@RiverHeart
Copy link
Author

After reviewing Azure/ResourceModules using the Microsoft.Web/sites/config resource seems like it solves the issue best since it takes an object. Also has the ability to be placed into a module.

var appSettings = {
  custom_key: 'value'
}
var appInsightsValues = !empty(appInsightId) ? {
  APPINSIGHTS_INSTRUMENTATIONKEY: appInsight.properties.InstrumentationKey
  APPLICATIONINSIGHTS_CONNECTION_STRING: appInsight.properties.ConnectionString
} : {}
var expandedAppSettings = union(appSettingsKeyValuePairs, appInsightsValues)

resource appSettings 'Microsoft.Web/sites/config@2022-03-01' = {
  name: 'appsettings'
  kind: kind
  parent: app
  properties: expandedAppSettings
}

Since the primary use case for this feature request is no longer relevant going to close this out.

@github-project-automation github-project-automation bot moved this from Todo to Done in Bicep Feb 15, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request Needs: Upvote This issue requires more votes to be considered
Projects
Archived in project
Development

No branches or pull requests

3 participants