search

jupyterhub / ldapauthenticator

LDAP Authenticator Plugin for Jupyter
BSD 3-Clause "New" or "Revised" License
202 stars 175 forks source link

ci: fix testing ldap server port mapping for broken gate #192

Closed bloodeagle40234 closed 3 years ago

bloodeagle40234 commented 3 years ago

Current CI gate has been broken because testing container image (rroemhild/test-openldap) changed the service ports from 389/636 to 10389/10636. (*1) That causes all existing test cases failed down as LDAP communication errors.

This pull request changes the service port according to the change, then that passed the CI successfully.

Closes https://github.com/jupyterhub/ldapauthenticator/issues/191

*1: https://github.com/rroemhild/docker-test-openldap/commit/adb4650727e0123c7c24c7d7ce8609d12384a29b

manics commented 3 years ago

Thanks for fixing this!

A big advantage of using ci/docker-ldap.sh instead of the GitHub workflow service is that it also works for local testing, and although the script is still there there's a danger it'll get out of sync with the workflow in future. Are there other advantages to using the GitHub service? If not would you mind switching back to the script? Thanks!

bloodeagle40234 commented 3 years ago

@manics Sorry, I didn't notice your reply so long. Your comment makes me sense so let's remove ff8053f to use ci/docker-ldap.sh.

Thanks.

bloodeagle40234 commented 3 years ago

I force-pushed the commit only for the port change

manics commented 3 years ago

Thanks!

welcome[bot] commented 3 years ago

Congrats on your first merged pull request in this project! :tada: congrats Thank you for contributing, we are very proud of you! :heart: