Skip to content

Conversation

andrewmooreio
Copy link

@andrewmooreio andrewmooreio commented May 13, 2025

Description

This PR adds a pool_maxsize parameter to the KeycloakOpenID and KeycloakAdmin classes. This parameter is passed to the internal ConnectionManager, allowing users to configure the maximum number of connections to keep in the connection pool.

Motivation/Context

Controlling the connection pool size is important for managing resource usage and improving performance in applications that make frequent calls to Keycloak. By exposing this parameter, users of the library can fine-tune the connection behaviour to better suit their environment and workload, potentially reducing overhead from establishing new connections.

Should resolve: #630

Testing Evidence

  • Added pool_maxsize parameter to relevant test fixtures in tests/conftest.py with a non-default value (5).
  • Added assertions in tests/test_keycloak_openid.py (test_keycloak_openid_init and test_a_well_known) to verify pool_maxsize is correctly set on the ConnectionManager for KeycloakOpenID.
  • Added assertions in tests/test_keycloak_admin.py (test_keycloak_admin_init and test_a_realms) to verify pool_maxsize is correctly set on the ConnectionManager for KeycloakAdmin.

Introduces the `pool_maxsize` parameter to `KeycloakOpenID` and `KeycloakAdmin` classes, allowing control over the underlying connection pool size in the `ConnectionManager`.

Adds corresponding tests to verify the parameter is correctly passed and stored for both synchronous and asynchronous clients.
@andrewmooreio
Copy link
Author

Note, the failed tests seem to be unrelated to this change and have been failing in CI for a while.

Copy link
Collaborator

@ryshoooo ryshoooo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @andrewmooreio

Thanks a lot for the changes! Overall looks good to me, I can't spot anything breaking. I've only added a comment about the tests, ideally we want to keep the fixtures as simple as possible, there is no need to set the pool size for each fixture.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep the fixtures minimal and just delete the pool_maxsize in here? (i.e. let the defaults take place). This way we can be sure that minimal configuration will keep working for most operations.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar here. There's no need to set the pool max size for all these cases. 1 time is enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to Configure Connection Pool Size in python-keycloak
2 participants