Update Azure SDK For Go#11849
Update Azure SDK For Go#11849mmorel-35 wants to merge 6 commits intoprometheus:mainfrom mmorel-35:github.com/Azure/azure-sdk-for-go
Conversation
discovery/azure/azure.go
Outdated
| return nil, errorNotFound | ||
| } | ||
| return nil, autorest.NewErrorWithError(err, "network.InterfacesClient", "Get", resp, "Failure responding to request") | ||
| return nil, autorest.NewErrorWithError(err, "armnetwork.InterfacesClient", "Get", nil, "Failure responding to request") |
There was a problem hiding this comment.
What would be the best way to get rid of autorest completly ?
There was a problem hiding this comment.
I think it depends on your logic about error, rely on autrest to wrap error is not necessary. Our new version of SDK does not provide utility about errors, mainly because the error returned by SDK is simple.
discovery/azure/azure.go
Outdated
| "github.com/Azure/go-autorest/autorest" | ||
| "github.com/Azure/go-autorest/autorest/adal" | ||
| "github.com/Azure/go-autorest/autorest/azure" |
There was a problem hiding this comment.
Those three dependencies should not be there anymore.
There was a problem hiding this comment.
The only left thing is error handling. As I said in previous comment, you could do anything with SDK returned error. I don't familiar with the error tracing framework of prometheus. If you need some help, feel free to give the detailed questions.
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
…m/mmorel-35/prometheus into github.com/Azure/azure-sdk-for-go
discovery/azure/azure.go
Outdated
| cloudConfig := cloud.Configuration{ | ||
| ActiveDirectoryAuthorityHost: env.ActiveDirectoryEndpoint, Services: map[cloud.ServiceName]cloud.ServiceConfiguration{ | ||
| cloud.ResourceManager: { | ||
| Audience: env.ServiceManagementEndpoint, | ||
| Endpoint: env.ResourceManagerEndpoint, | ||
| }, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Hi @tadelesh
Shall we replace the azure.EnvironmentFromName(...) with a map for cloud.Configuration like this one ?
var environments = map[string]Environment{
"AZURECHINACLOUD": cloud.AzureChina ,
"AZUREGERMANCLOUD": cloud.AzurePublic ,
"AZURECLOUD": cloud.AzurePublic ,
"AZUREPUBLICCLOUD": cloud.AzurePublic ,
"AZUREUSGOVERNMENT": cloud.AzureGovernment ,
"AZUREUSGOVERNMENTCLOUD": cloud.AzureGovernment ,
}
Or Is there an already existing way to map this ?
There was a problem hiding this comment.
We do not use such environment any more in new SDK version. We have defined three common clouds' configuration in azcore. You have already used it: cloud.AzureGovernment, cloud.AzureChina, cloud.AzurePublic. I don't know whether you have exposed such environment settings to customer in your code, if so, then the mapping is fine to keep consistency, else, I think you could use cloud configuration directly.
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
|
Replaced by #11860 |
Hi @xboxeer ! Hi @roidelapluie ! Hi @sandeep-sen !
This is a first attempt to fix #10942
With the help of the migration guide and the documentation of the different documentation I'm proposing this solution.
But two things are bothering me. First I add to create a Policy to override the user-Agent. Second, I had to split the Network Interface ID to extract resourceGroupName and networkInterfaceName so I can call the Get method of the NetWorkInterfacesClient.
The second method is specially risky in case of the evolution of the endpoint structure.
Do you have any idea to make this more elegant and maintainable ?
Fixes #10942
Signed-off-by: Matthieu MOREL matthieu.morel35@gmail.com