resource_group: remove data source + location required

[DeviaVir]

Describe your changes

I went through the git history and couldn't find a reason as to why we are using a data source here, using a data source assumes that a resource already exists. For a greenfield where the resource_group is being created in the same terraform apply this data source will always error out.

This change removes the data source and relies on the user input to be correct.

This is a behavior change that may have unexpected side-effects that I'm maybe not seeing, curious to hear your thoughts.

Issue number

#602

Checklist before requesting a review

• The pr title can be used to describe what this pr did in CHANGELOG.md file
• I have executed pre-commit on my machine
• I have passed pr-check on my machine

Thanks for your cooperation!

[DeviaVir]

@microsoft-github-policy-service agree

[lonegunmanb]

Hi @DeviaVir thanks for opening this pr to us. This data source was introduced very early, this change allows users to pass resource group name only without location. Removing such data source would be considered as a breaking change since the resource group's location would be a required input then. I agree with you that we should get rid of this data source. I'll consider this request as an update in our next major version upgrade.

[DeviaVir]

Hi @DeviaVir thanks for opening this pr to us. This data source was introduced very early, this change allows users to pass resource group name only without location. Removing such data source would be considered as a breaking change since the resource group's location would be a required input then. I agree with you that we should get rid of this data source. I'll consider this request as an update in our next major version upgrade.

Thanks for this context, I'll do a bit of testing - if the idea is that we want the module to be optionally called without a location ("to be inferred") we may add some count logic.

[DeviaVir]

@lonegunmanb this change still works for greenfields (just verified) and should also work for existing users that do not pass a location to the module.

[lonegunmanb]

I would prefer a simple way, we should not create resource group in this module, nor should us query it, all information related to the resource group and location should be injected into this module.

This data source would be removed, and a location would be required in next major version.

[DeviaVir]

I would prefer a simple way, we should not create resource group in this module, nor should us query it, all information related to the resource group and location should be injected into this module.

This data source would be removed, and a location would be required in next major version.

I don't disagree, however doing it this way means it is not a breaking change and safe to merge in a next minor version. If that's not interesting, I'd be happy to roll this back to just removing the data source and resource group creation and wait for the major version. Let me know what you want to do.

[lonegunmanb]

Thanks @DeviaVir for the update, I've reviewed your pr, it improves this module by cancelling data source query when the location has been provided. After evaluating the module, could you please modify this pr by deleting the data source block? I'd like to include this pr as part of our next major version, which might be soon.

[DeviaVir]

Thanks @DeviaVir for the update, I've reviewed your pr, it improves this module by cancelling data source query when the location has been provided. After evaluating the module, could you please modify this pr by deleting the data source block? I'd like to include this pr as part of our next major version, which might be soon.

Done. I have additionally removed the default value for var.location which means that those calling the module without any value for location will see an error and thus be made aware that there's been a breaking change that impacts them.