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

Added Environment param to use other instances #14

Merged
merged 1 commit into from
Jun 21, 2022
Merged

Added Environment param to use other instances #14

merged 1 commit into from
Jun 21, 2022

Conversation

SamErde
Copy link
Contributor

@SamErde SamErde commented Apr 15, 2022

Added the Environment parameter to allow users to connect to other instance types, including Global (default), China, Germany, USGov, and USGovDod. Future improvements can use the Get-MgEnvironment commandlet to pull and validate the current list of instance names.

Added the Environment parameter to allow users to connect to other instance types, including Global (default), China, Germany, USGov, and USGovDod. Future improvements can use the Get-MgEnvironment commandlet to pull and validate the current list of instance names.
@merill merill merged commit f26bdca into microsoft:main Jun 21, 2022
@merill
Copy link
Contributor

merill commented Jun 21, 2022

Thanks for the reminder @SamErde

Your PR has been merged and published to the PowerShell gallery 🙏

Cheers

@SamErde
Copy link
Contributor Author

SamErde commented Jun 21, 2022

Thanks, @merill! I'm curious if you have any preference about a potential optimization of this function:

In this PR, I hard coded the known instance names. However, the Get-MgEnvironment cmdlet can be used to query the Graph API for a list of Azure environment names in case any are changed or added in the future. Would it be preferable to use this each time to ensure that the Connect-AzureADExporter function is always aligned with those names?

image

@merill
Copy link
Contributor

merill commented Jun 21, 2022

Agree on making it dynamic but that would remove the validateset for the parameter and make it harder for users since it cannot do auto fill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants