Aysnc methods on resource clients do not use standard Async naming conventions

[SiliconKid]

Hi

I've been working with this library extensively over the last week and it works very well and I'm very happy with the capabilities.

The only thing that I feel is worth commenting on is the non standard naming of async methods.

This has now bitten me several times because I have failed to realise that I needed to await a method and then couldn't understand why I was seeing bizarre things happen, only to discover that instead of retrieving an object of given type I had a actually retrieved a Task which was still waiting to be executed.

This is a consistent problem across all of the resource clients as far as I can see.

The method names should have the word Async as a suffix to their names to avoid this confusion.

Example

var existingService = await kubeClient.ServicesV1().Get(k8sService.Metadata.Name, k8sNamespace);

Should be:

var existingService = kubeClient.ServicesV1().GetAsync(k8sService.Metadata.Name, k8sNamespace);

That Get method actually returns a Task and Get is Async, so I should be awaiting that Get call if I want to get back the actual existing service object (or null if one does not exist).

This is especially dangerous because if you no Await that method, you will NOT get back null, even IF the service does NOT exist. You get back a Task, which is non null. Which means that if you have a null check directly after that call that you are using to determine if the service exists already, that check will see a non null object and falsely report that the service DOES exist.

All because Get is Async and needs to be Awaited to get the actual result, which should be null.

This is common to all Async methods on all the resource clients from what I can see.

Would be great if the lib could be updated to standardize the naming conventions as per industry standards. I'm sure it would avoid a lot of confusion going forward.

Other than that, great lib which is extremely useful and generally easy to use.

Allan

[tintoy]

Hi - thanks for the feedback!

I’ll try to have a proper look at this sometime on Thursday.

[tintoy]

Hey, thanks for taking the time to leave such detailed feedback - sorry for the late response, but work has been a little crazy recently.

I guess I'd never really noticed that particular problem because I've never run into it; I haven't tried using var in that way, but I can see how the issue could be tricky to spot in that scenario. I'm sorry the API design caught you out like that; I usually try to think carefully about API-design footguns and avoid them where practical. I previously considered changing the naming of async methods but it would represent a breaking change to existing users and I try hard to avoid that if it all possible.

If it helps, the code you specified in your example should generate compiler warning CS4014; I appreciate that it's not ideal, but it's better than nothing 🙂

Glad to hear you find it useful, BTW!

[SiliconKid]

Hi Adam

I think the ideal scenario here is to provide async method signatures with the standard Async suffix but leave the existing methods without the suffix so as not to cause breaking changes. Just make the Async methods call the ones without the suffix so there's no code duplication.

That way people will at least see the method signatures with the Async convention in intellisense and can use those, but under the hood both variants of the method are actually async and will work.

Allan