deprecation: package needs updating

[squillace]

[itowlson]

Last time I looked at this, @azure/arm-containerservice didn't actually work for us - I think it didn't play nicely with the credentials that the Azure Account extension gave us. I think I raised an issue for that but I can't remember where or find it now... anyway @Tatsinnit has bravely volunteered to pick this one up so hopefully he will be able to get it sorted!

[Tatsinnit]

👍 Thank you so much: @itowlson - I might have a fix for this, I might share\show\thought-share tomorrow as a fix for this, at least at this repo level. 🙈 (Linked Draft PR for your eyes) #23

[Tatsinnit]

💡 I am re-opening this, because I can see with the changes type fix we end up in a internal error for MS-Rest-JS in --> ServiceClient.ts ==> sendRequest which cause extension level type error. Thanks heaps.

• Notes: Some comments in this behaviour mentioned in the following link stands true for this extension as well: Compatibility with new Azure SDK microsoft/vscode-azure-account#140

i.e. even if I do the type morphing the ServiceClientCredentials for will vary, and essentially when ms-rest-js reaches the senRequest I see an obscure i.e. not so helpful message so not sure what I can gather from that.

[Tatsinnit]

Hiya guys @philliphoff & @itowlson 🛡️ 🤖

Below is my 2 cent finding of how this update will break existing merge config and save kubeconfig fucntionality.

Weird ms-rest-js ==> serviceClient.ts failure led to more deep dependency issue.

I am sure new package is bigger and better, butI really think that this whole @azure/containerservices or arm-resource et. al. packages change was not thought through and they contain breaking changes from the original packages, and mere: https://github.com/Azure/azure-sdk-for-js/blob/master/documentation/Migration.md is not full thought through for various other dependencies 🐉

Here is the weird behaviour I see so I am planning to revert my changes and keep using old packages until we have clear plan to tackle this. Please advice.

• Behaviour after Package update: When I select things like save kubeconfig I see random error thrown which is like this: TypeError: Cannot read property 'then' of undefined which gets thrown from this line: https://github.com/Azure/vscode-aks-tools/blob/master/src/extension.ts#L52

Lets Dig Deeper

• We use vscode-azureextensionui which has ISubscriptionContext dependency which we use for our credential to be types as ServiceClientCredentials Now this is dependent on ms-rest i.e. NOT ms-rest-js :) and surely ms-rest-js has done this differently in new implementation because I see signRequest has different return signature + even simple ugly type conversation let it fail at the ms-rest-js ==> ServiceClient.ts on this line https://github.com/Azure/ms-rest-js/blob/master/lib/serviceClient.ts#L328 . https://github.com/Azure/vscode-aks-tools/blob/master/src/tree/azureAccountTreeItem.ts#L5

• Gyst: I noticed that when we upgraded our packages to latest @azure/arm-containerservice **Why ms-rest-js fails at ms-rest-js ==> sendRequest then I realised that might have to do with the type conversion. Which points that ms-rest-js have some sort of different way of handling the sendRequest vs ms-rest. 🐓

Plausible solution with a But :

• Seems like only way to correctly use is to Change the depenedent on using alternative to using vscode-azureextensionui because the ServiceClientCredentials from this is dependent on ms-rest (that is deep, and do we have and alternative at all for this) - I think not at this stage.

To unblock this for now:

• I propose we keep using the old one until be have a plan or you guys think that there is an easy alternative to the types we used at clusterTreeItem level etc...

Thanks heaps in taking time for reading this, I thought I will do a quick idea dump as to what are my findings and get your pinions to move forward for this. : = )

[Tatsinnit]

💡 -- Thanks @itowlson for quick chat and you summed it well, i..e bunch of stuff not talking nicely to each other with this extension is consuming so I am going ahead and reverting these changes.