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

LDAP3: Allow the user to set the search filter. #100

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

thinkl33t
Copy link
Contributor

This should allow us to only sync a subset of users.

Closes: #98

@thinkl33t
Copy link
Contributor Author

have tested this locally - works well :)

@jonathanmaw
Copy link
Contributor

I agree that the changes work, though I'm a little concerned that we can add these changes without needing to reflect any of it in documentation or tests.

It seems that we don't have an example config for the ldap3 source, though a user would be given a hint that the fields exist because apidoc pulls out the value of optional_fields.

I'm happy to approve this as-is, if we create an issue about the test coverage and documentation gap for when we have the development bandwidth to cover them.

@thinkl33t
Copy link
Contributor Author

#103 and #104 added to cover the gaps

Copy link
Contributor

@jonathanmaw jonathanmaw left a comment

Choose a reason for hiding this comment

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

With #103 and #104 to catch testing and documentation gaps, I'm happy for this to be merged.

This should allow us to only sync a subset of users.

Closes: #98
@thinkl33t thinkl33t force-pushed the bobclough/ldap-add-search-filters branch from 2a6056a to 1b49689 Compare December 6, 2023 11:00
@thinkl33t thinkl33t merged commit 1d13021 into main Dec 6, 2023
7 checks passed
@thinkl33t thinkl33t deleted the bobclough/ldap-add-search-filters branch December 6, 2023 11:04
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.

LDAP3: allow the user to set their search filter
2 participants