Async feature

[pidaiman]

Add support for async APIs using httpx client without interfering with the original functionality.

Linked issue: https://github.com/marcospereirampj/python-keycloak/issues/337

[pidaiman]

Hello @ryshoooo,

I have added the additional packages to the pyproject.toml file and revised the commit messages to pass the first check. However, the second check continues to prompt me to update the poetry.lock file, despite having done so multiple times. Could you provide any insights into what might be causing this issue and suggest potential resolutions?

[ryshoooo]

Hello @ryshoooo,

I have added the additional packages to the pyproject.toml file and revised the commit messages to pass the first check. However, the second check continues to prompt me to update the poetry.lock file, despite having done so multiple times. Could you provide any insights into what might be causing this issue and suggest potential resolutions?

Hi @pyaiman

Looks like there is some issue with the docutils package resolution. Should be fixed now, there should be only linter errors now :)

[pidaiman]

Hi @ryshoooo,

I have created this new PR due to commit history conflict with the previous branch.

As you mentioned there to enforce the creation of async counterparts. I can probably make a test which will compare sync with their async counterparts as the nomenclature is very convenient for that.

Thanks for your efforts.

[pidaiman]

Hi @ryshoooo ,

The check-docs check is failing due to some module not found. Looks like a build error. Can you confirm? Thanks

[ryshoooo]

Hi @ryshoooo ,

The check-docs check is failing due to some module not found. Looks like a build error. Can you confirm? Thanks

Should be solved now

[ryshoooo]

@pyaiman

Looking good :) I've added a couple of missing async calls in openid and uma clients, that could potentially block the thread, depending on the async runtime.

Let's add the divergence test on the sync vs async methods (i.e. each async should have its accompanying sync method exist in each client and vice versa) and documentation update, that should be then good to go :)

[ryshoooo]

@pyaiman There is a slight behavioral change introduced though. Now we no longer fetch the access token upon initialization of KeycloakOpenIdConnection client. Mainly because we don't know which method to use (sync or async one) and using the wrong one can lead to very bad stuff in real (especially async) applications. The access token gets automatically fetched when calling any of the action methods in OID or Admin clients, so I don't consider this a breaking change. It will break some of my applications though as I expect these tokens to be immediately there after initialization, but I'd honestly say that's incorrect usage of this library. If you want a token out, be explicit and make the get_token or await a_get_token call in the application.

[pidaiman]

Hi @ryshoooo , Thank you for pointing out and the changes. As far as I have seen the token is fetched first in the function and after that the request is executed. As we are using await for the a_get_token wouldn't that behave the same as get_token?

Also about the documentation, I have added the docstrings for each async function. Is there anything else required from my side regarding documentation ?

[ryshoooo]

Hi @ryshoooo , Thank you for pointing out and the changes. As far as I have seen the token is fetched first in the function and after that the request is executed. As we are using await for the a_get_token wouldn't that behave the same as get_token?

Also about the documentation, I have added the docstrings for each async function. Is there anything else required from my side regarding documentation ?

It is fetched first in the function, but not in an async way. Which could be blocking if used in an async framework such as tornado for example (speaking from experience here). It is therefore safer to make sure that all HTTP requests with the async client only happen in the async runtime rather than the main thread.

For the documentation, I think it would be brilliant to add an async.rst module into https://github.com/marcospereirampj/python-keycloak/tree/master/docs/source/modules and link it in https://github.com/marcospereirampj/python-keycloak/blob/master/docs/source/getting_started.rst?plain=1. Just showing how can a user create an async client and use the async methods. The docstrings themselves are fine, but we also have these restructured text doc files for quick start and simple examples, usually easier to digest than the API docs :)

[pidaiman]

Hi @ryshoooo

I have included the docs for async. Let me know if anything else is required. Thanks again for your efforts and assistance.

[jucasansao]

@ryshoooo I do think the change of behaviour is a very relevant change. It makes sense to assume that once the admin is initialised, the token is generated right away. My code for example had a verification that the token was indeed created after initialising the admin (as I do with every initialization), before I proceeded with the remaining code.

Why not have a default of sync, and add an argument for async=True?

[ryshoooo]

@ryshoooo I do think the change of behaviour is a very relevant change. It makes sense to assume that once the admin is initialised, the token is generated right away. My code for example had a verification that the token was indeed created after initialising the admin (as I do with every initialization), before I proceeded with the remaining code.

Why not have a default of sync, and add an argument for async=True?

Hi @jucasansao

Thanks for the input :) I also have apps that expect the admin object to be initialized with a token, and as useful as it is, it also gave me headaches (especially in testing, which then resulted in the necessity to mock the KeycloakAdmin object everywhere). It's just something I've been thinking about for a while, but never got to change as there wasn't really a good case against it, other than my personal annoyance.

I was thinking about a parameter to the init method as a solution, exactly what you've described, but the problem is that we can't use await in the __init__ method (as it's not a coroutine), meaning we would have to make the assumption of how the coroutine should be scheduled and where. That's usually defined by the framework you are using, making it a bit outside of the scope of this package. Perhaps it could work with both parameters async=True, async_run=asyncio.run and have if async: async_run(self.a_get_token) or similar, which is something I've considered as well, but decided against, as we are essentially sending data to the init method of the client, that do not define the object (i.e. these are not properties/attributes) but they define the code to run instead. So then maybe just run the code yourself instead of sending it to the object to execute it for you:)

I'm happy to hear counter arguments, but eventually since all actionable methods keep on working with this change (the token gets fetched before the execution) and we do not do any network communication upon initialization, I gravitated towards the solution we have in this MR as the better way to go :) This change would really only effect people that expect the tokens to be there after init. Which is in my opinion a bit magical. What I mean is if I see a code like:

admin = KeycloakAdmin(...)
tokens = admin.token

I'd expect tokens to be empty, but they are not. However with code like

admin = KeycloakAdmin(...)
admin.get_token()
tokens = admin.token

I'd expect the tokens to be filled in. It's more clear, explicit, and gives you the power to decide when the tokens should be fetched, which is actually something I prefer :)