-
Notifications
You must be signed in to change notification settings - Fork 745
Updated versions for Azure.Provisioning and Azure.Provisioning.AppSer… #13281
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
base: main
Are you sure you want to change the base?
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13281Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13281" |
| output serviceBusEndpoint string = messaging.properties.serviceBusEndpoint | ||
|
|
||
| output name string = messaging.name | ||
| output name string = 'existingResourceName' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ArcturusZhang - is this change expected?
The code that is producing this is:
| var resource = AzureProvisioning.ServiceBusNamespace.FromExisting(identifier); | |
| resource.Name = name; | |
| return resource; |
and
| // We need to output name to externalize role assignments. | |
| infrastructure.Add(new ProvisioningOutput("name", typeof(string)) { Value = serviceBusNamespace.Name }); |
so resource.Name is being set to a hard coded BicepValue<string>, and then the ProvisioningOutput's Value is being set to the serviceBusNamespace.Name, which was set above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if we always want an expression, we now must call the ToBicepExpression() on the BicepValue instance.
Previously whether outputting a value or an expression is kind of based on an internal rule which varies from types, and the beta version made a unification on that rule.
Now - a IBicepValue instance will always spit out its literal value if it has a literal value, also always spit out its expression value if it is assigned with an expression.
I think here from the code you are sharing, and if to spit out whatever in the bicep value is your intention, this is expected.
And from the result, the new bicep should be identical with the original one.
But if your purpose here is to always reference the property, I suggest we add a ToBicepExpression invocation which makes you always get a messaging.name expression
| failoverPriority: 0 | ||
| } | ||
| ] | ||
| capabilities: [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ArcturusZhang - is this expected? Is this going to cause any problems, for example, if there are capabilities on the resource, is this going to clear it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you show me the code that builds this part?
We have a behavior change that previously we never write an empty array, now we write an empty array when the array was assigned explicitly (touched)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And as far as I know, a resource written as this, would always override whatever we have on azure. Therefore the original bicep would also erase the capabilities property, making this bicep identical as before.
But as I said above, if you could share the code that outputting this, I could check if it is expected in your usage to write this empty array because it does not always write an empty array right now.
| "type": "string", | ||
| "pattern": "^-?(\\d{1,7}|((\\d{1,7}[\\.:])?(([01]?\\d|2[0-3]):[0-5]?\\d|([01]?\\d|2[0-3]):[0-5]?\\d:[0-5]?\\d)(\\.\\d{1,7})?))$", | ||
| "description": "The timeout applied to an individual network operations." | ||
| "description": "The timeout applied to individual network operations." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These came from
…vice packages
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplatebreaking-changetemplatediagnostictemplate