Docker.DotNet icon indicating copy to clipboard operation
Docker.DotNet copied to clipboard

Get token from AuthenticateAsync

Open Emdot opened this issue 4 years ago • 4 comments

What version of Docker.DotNet?:

fork from main as of 2/14/2021 (commit 8e3cf4f7)

Problem: ISystemOperations.AuthenticateAsync does not provide a way for the caller to get the token that was returned in the REST response.

Its signature is Task AuthenticateAsync(AuthConfig authConfig, CancellationToken cancellationToken = default(CancellationToken)), but it should probably be Task<AuthResponse> AuthenticateAsync(AuthConfig authConfig, CancellationToken cancellationToken = default(CancellationToken)) instead.

Other details: I'm happy to submit a PR, but what's this project's preferred way to handle changes that would break binary compatibility? Just changing the return type would be breaking. An overload with an out parameter isn't possible because it's an async method. Are you OK with adding an Authenticate2Async method or similar?

Emdot avatar Feb 17 '21 02:02 Emdot

@Emdot I'm glad you brought this here. Same happens on endpoints returning Task instead of existing Task < DockerApiResponse >

I'm not a maintainer, but I would like to see your approach on this. Regarding breaking binary compatibility, it should get fixed by rebuilding applications with no required changes on code I believe

I encourage you to do it for the sake of having fun

dgvives avatar Feb 17 '21 22:02 dgvives

Unfortunately, I know too many ways to fix API versioning problems, and none of them are ideal. My approach would depend firstly on how willing we are to issue a new major version at this point, secondly on whether we're aiming for binary compatibility vs source compatibility, and thirdly on whether we support the possibility that consumers may have custom implementations of IContainerOperations interface. All of those questions require a maintainer to weigh in. :)

Emdot avatar Feb 18 '21 01:02 Emdot

@Emdot - We have not released a version in a very long time for lots of reasons I'm not getting into haha. Please feel free to overwrite this method and have it be the correct signature that it should have been originally. When @galvesribeiro does a release we will do a major version bump.

jterry75 avatar Feb 18 '21 17:02 jterry75

We can set a release soon with the current codebase if you feel its ok.

If you guys want to fix the method overload as @jterry75 mentions, just feel free to open a PR and we can push the release right after.

galvesribeiro avatar Feb 18 '21 17:02 galvesribeiro