Skip to content
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

Added unbind_s, search_ext and result3 support; changed rename_s to work how python-ldap does #22

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

cmalek
Copy link

@cmalek cmalek commented Feb 3, 2021

Hello

Thank you for your fakeldap module!

I've added a few things that we needed in order to test our code, and also did some PEP8 fixes to make my linter happy.

  • Added unbind_s support
  • Added search_ext and result3 support
  • Changed rename_s to work how python-ldap expects: rename_s(dn, newrdn, superior)
  • When using the argument list as a dict key, serialize the argument list with a bytes-aware JSON encoder. This fixes the "list is unhashable" , "bytes are unhashable", etc. problems

best wishes
Chris

@timabbott
Copy link
Member

If you can, it'd be nice if you can clean up the commits to follow the Zulip commit guidelines: https://zulip.readthedocs.io/en/latest/contributing/version-control.html#commit-messages

We're less strict about them with this project (which we adopted some time ago), but I think it'd be quite helpful for reviewing this efficiently to have real commit messages.

@mateuszmandera I assume you'll take point on reviewing this (we should at the very least make sure it doesn't break the zulip/zulip test suite).

@cmalek
Copy link
Author

cmalek commented Feb 16, 2021

Will do! Thank you for replying! I'd been using mock and patch on my LDAP test code for years, and using fakeldap is sooo much cleaner, so I appreciate your work here.

@timabbott
Copy link
Member

@cmalek just a bump on cleaning up the commit history.

@dill0wn
Copy link

dill0wn commented May 16, 2022

@cmalek Just a bump on this -- our team would be very happy to see this functionality merged :)

@timabbott
Copy link
Member

This is a bit late of a reply, but @dill0wn or anyone else who comes across this, feel free to submit a new PR that cleans this up.

@cmalek
Copy link
Author

cmalek commented Mar 1, 2023

I actually ended up writing my own project for this that did more of what I needed. https://python-ldap-faker.readthedocs.io/en/latest/

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.

3 participants