-
Couldn't load subscription status.
- Fork 712
Add Azure provisioning command handling and settings configuration #10038
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
Conversation
Improve user prompt for Azure subscription settings in provisioning context Update Azure subscription prompt to include HTML link for creating a free account
| new InteractionInput { InputType = InputType.SecretText, Label = "Subscription ID", Placeholder = "Select Subscription ID", Required = true }, | ||
| new InteractionInput { InputType = InputType.Text, Label = "Resource Group", Value = "GetResourceGroupDefault"}, | ||
| ], | ||
| new InputsDialogInteractionOptions { ShowDismiss = false, EnableMessageMarkdown = true }, |
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.
It should be possible to dismiss, allowing users to interact with the dashboard and other resources still.
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.
It is possible.
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.
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.
Can you also query for subscription ids?
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.
I can, but doing that would prevent me from entering in something that isn't in the list. I plan on openining an interaction service feature request tomorrow to support an input that allows both a list of options and the ability to enter in a free form value.
Without this, do you think for 9.4 we should leave it as-is without the list? Or should we query the for the list and show it if we found some (preventing entering something that isn't in the list)?
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.
Are values remembered? A checkbox was added to the parameters dialog for that.
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.
@eerhardt yes we need to save them to user secrets
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.
It auto persists to user secrets, which I think is a good default (all of the other azure state does as well).
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.
@eerhardt lets update the link to description to point to an aka.ms link. We can remove the Azure free account link and just use one that points to the docs.
cc @IEvangelist
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.
It auto persists to user secrets, which I think is a good default (all of the other azure state does as well).
At one point I had a checkbox (which defaulted to true), but I removed it. My reasoning was because if you didn't save the information, when you added another Azure resource on a new run, you would get prompted again and if you choose a different Resource Group, or subscription, you'd get into a weird torn state where some resources were in one location/resourcegroup/sub and some where in another.
I think it is better to just always save the information. So you can't get into this state as easy.
- Save data to user secrets
src/Aspire.Hosting.Azure/Provisioning/Internal/DefaultProvisioningContextProvider.cs
Show resolved
Hide resolved
src/Aspire.Hosting.Azure/Provisioning/Internal/DefaultProvisioningContextProvider.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure/Provisioning/Internal/DefaultProvisioningContextProvider.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure/Provisioning/Internal/DefaultProvisioningContextProvider.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure/Provisioning/Internal/DefaultProvisioningContextProvider.cs
Outdated
Show resolved
Hide resolved
| { | ||
| while (_options.Location == null || _options.SubscriptionId == null) | ||
| { | ||
| var locations = typeof(AzureLocation).GetProperties(BindingFlags.Public | BindingFlags.Static) |
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.
Do we need to do this more than once ever?
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.
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.
At least it is public reflection - and not the private reflection you had originally. 😆
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.
It looks like you should first prompt for subscription, and then from a subscription you can get the list of locations:
var armClient = new ArmClient(credential);
// Get the subscription (replace with your actual subscription ID)
var subscription = await armClient.GetDefaultSubscriptionAsync();
// List all locations (regions) available for this subscription
await foreach (var location in subscription.GetLocationsAsync())
{
Console.WriteLine($"{location.Name} - {location.DisplayName}");
}Need to refactor InteractionResult to allow for mocking without using IVT.
|
I believe this is ready for review now. FYI - @davidfowl @JamesNK I had to make some changes to the public API of InteractionResult since it isn't possible to test users of the InteractionService without using IVT. |
|
👍🏾 ship it |
src/Aspire.Hosting.Azure/Provisioning/Internal/DefaultProvisioningContextProvider.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure/Provisioning/Internal/DefaultProvisioningContextProvider.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure/Provisioning/Internal/DefaultProvisioningContextProvider.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure/Provisioning/Internal/DefaultProvisioningContextProvider.cs
Outdated
Show resolved
Hide resolved
| EnableMessageMarkdown = true, | ||
| ValidationCallback = static (validationContext) => | ||
| { | ||
| var subscriptionInput = validationContext.Inputs[1]; |
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.
Instead of referencing inputs by index, you could create them earlier and assign them to variables, use the variable in the array passed to PromptInputsAsync and here in the validation. The instance will be the same. It's non-obvious that you can do that.
Although I'm starting to wonder if inputs should have a name and be accessible via name here and in the result...
Keep referencing by index and in 9.5 I'll change the API to make inputs a KeyedCollection.
src/Aspire.Hosting.Azure/Provisioning/Internal/DefaultProvisioningContextProvider.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Hosting.Azure/Provisioning/Internal/DefaultProvisioningContextProvider.cs
Outdated
Show resolved
Hide resolved
| if (!IsValidResourceGroupName(resourceGroupInput.Value)) | ||
| { | ||
| validationContext.AddValidationError(resourceGroupInput, "Resource group name must be a valid Azure resource group name."); | ||
| } |
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.
This doesn't say why the name isn't valid. It's not a good user experience.
| await validationCallback(context).ConfigureAwait(false); | ||
|
|
||
| return context.HasErrors; | ||
| return !context.HasErrors; |
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.
Was this a bug?
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.
Yep
Co-authored-by: James Newton-King <[email protected]>
|
/backport to release/9.4 |
|
Started backporting to release/9.4: https://github.com/dotnet/aspire/actions/runs/16232571461 |

Description
Use the new interaction service to prompt in the dashboard for subscription id and location.
Checklist
doc-ideatemplatebreaking-changetemplatediagnostictemplate