-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add option pool_lifetime option to ldap #395
base: master
Are you sure you want to change the base?
Conversation
I tested this PR rebased onto v8.0.1 and found no issues. Thank you for this nice commit. |
@shaardie Would you have time to add a test for this functionality? The limited testing we have for the LDAP attribute store is in tests/satosa/micro_services/test_ldap_attribute_store.py. It would be great to see the tests evolve. |
Yes, I can not see that the other parameter used in the ldap3 connection are tested. Otherwise I would have just followed the test strategy and extended it. I think this is a more general problem in this microservice and should be tackled outside of the scope of this particular Pull Request. Unfortunately, I will not have time the next weeks to write such tests. But explicitly for this parameter: We are running this already successful in production for quite some time. |
Any update here? This PR is approved for quite some time now. |
This patch adds another option to the ldap connection. Next to the other pool connections, it is now possible to set the `pool_lifetime`.
This patch adds tests to check the configuration of the ldap connection. Signed-off-by: Sven Haardiek <sven@haardiek.de>
ce98675
to
97cbdf8
Compare
Hej @skoranda, I added a test to test different configs set on the I am not an expert in writing tests for Python code. Is this sufficient? If yes, I maybe try to add tests for the other ldap PR I wrote. |
@shaardie just a quick note to let you know I saw this and will try to get to it in the next two weeks. Thanks. |
Any progress on this one? |
This patch adds another option to the ldap connection. Next to the other pool
connections, it is now possible to set the
pool_lifetime
.Some LDAP Server abandon connections after some time without notifying the client. This option allow to set a maximal pool lifetime, to also close connections on the client.
All Submissions: