Skip to content

Allow passing None as uri argument to ldap.initialize()#465

Merged
droideck merged 1 commit intopython-ldap:mainfrom
steffen-kiess:allow-ldap_initialize_None
Aug 6, 2025
Merged

Allow passing None as uri argument to ldap.initialize()#465
droideck merged 1 commit intopython-ldap:mainfrom
steffen-kiess:allow-ldap_initialize_None

Conversation

@steffen-kiess
Copy link
Contributor

@steffen-kiess steffen-kiess commented Mar 24, 2022

OpenLDAP allows passing NULL to ldap_initialize(). In this case the
default URI from ldap.conf will be used. Allow passing None as uri
argument to ldap.initialize().

@mistotebe
Copy link
Contributor

mistotebe commented Apr 6, 2022

LGTM, can you also update Doc/reference/ldap.rst?

1 similar comment
@mistotebe
Copy link
Contributor

mistotebe commented Apr 6, 2022

LGTM, can you also update Doc/reference/ldap.rst?

@steffen-kiess steffen-kiess force-pushed the allow-ldap_initialize_None branch from c091f7f to 0c6d462 Compare April 6, 2022 12:11
@steffen-kiess
Copy link
Contributor Author

steffen-kiess commented Apr 6, 2022

LGTM, can you also update Doc/reference/ldap.rst?

I've updated the pull request.

@mistotebe mistotebe force-pushed the allow-ldap_initialize_None branch from 0c6d462 to 9d6ce80 Compare April 6, 2022 13:21
mistotebe
mistotebe previously approved these changes Apr 6, 2022
View reviewed changes
@droideck
Copy link
Contributor

droideck commented Aug 31, 2022

@steffen-kiess could you please rebase the PR with the current master branch?

droideck
droideck previously approved these changes Sep 1, 2022
View reviewed changes
Copy link
Contributor

@droideck droideck left a comment

Choose a reason for hiding this comment

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

Otherwise, looks good to me too. :)

@steffen-kiess steffen-kiess dismissed stale reviews from droideck and mistotebe via 7e6c915 September 2, 2022 09:45
@steffen-kiess steffen-kiess force-pushed the allow-ldap_initialize_None branch from 9d6ce80 to 7e6c915 Compare September 2, 2022 09:45
@steffen-kiess
Copy link
Contributor Author

steffen-kiess commented Sep 2, 2022

@steffen-kiess could you please rebase the PR with the current master branch?

I've rebased the commit.

@mistotebe
Copy link
Contributor

mistotebe commented Sep 2, 2022

@steffen-kiess Reviewing the PR history, some parts of the documentation have gone from the original commit 9d6ce80, is that intentional?

@steffen-kiess steffen-kiess force-pushed the allow-ldap_initialize_None branch from 7e6c915 to c4892e9 Compare September 2, 2022 12:58
@steffen-kiess
Copy link
Contributor Author

steffen-kiess commented Sep 2, 2022

@steffen-kiess Reviewing the PR history, some parts of the documentation have gone from the original commit 9d6ce80, is that intentional?

Sorry, no, that was not intentional. I rebased the wrong commit (the one I pushed, not the one updated by you). I've pushed a new rebased commit with the documentation changes.

@steffen-kiess
Copy link
Contributor Author

steffen-kiess commented Dec 15, 2022

@mistotebe @droideck Is there anything else I should be doing for this PR?

mistotebe
mistotebe previously approved these changes Dec 15, 2022
View reviewed changes
droideck
droideck previously approved these changes Dec 15, 2022
View reviewed changes
Copy link
Contributor

@droideck droideck left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks!

@tiran
Copy link
Member

tiran commented Nov 6, 2023

I would prefer if you keep the uri argument mandatory and don't default to None. That would require users to explicitly opt-in for default argument with SimpleLDAPObject(None) instead of SimpleLDAPObject().

@steffen-kiess steffen-kiess dismissed stale reviews from droideck and mistotebe via d84deff November 6, 2023 13:37
@steffen-kiess steffen-kiess force-pushed the allow-ldap_initialize_None branch from c4892e9 to d84deff Compare November 6, 2023 13:37
@steffen-kiess
Copy link
Contributor Author

steffen-kiess commented Nov 6, 2023

I would prefer if you keep the uri argument mandatory and don't default to None. That would require users to explicitly opt-in for default argument with SimpleLDAPObject(None) instead of SimpleLDAPObject().

Ok, I've changed that so that uri stays mandatory. (And I also rebase to the current main branch).

@droideck
Copy link
Contributor

droideck commented Aug 6, 2025

@steffen-kiess hi! Could you please rebase? I think we should be ready to merge it after that.

@steffen-kiess
Allow passing None as uri argument to ldap.initialize()
a0c2c7d 
OpenLDAP allows passing NULL to ldap_initialize(). In this case the
default URI from ldap.conf will be used. Allow passing None as uri
argument to ldap.initialize().
@steffen-kiess steffen-kiess force-pushed the allow-ldap_initialize_None branch from d84deff to a0c2c7d Compare August 6, 2025 17:18
@steffen-kiess
Copy link
Contributor Author

steffen-kiess commented Aug 6, 2025

@steffen-kiess hi! Could you please rebase? I think we should be ready to merge it after that.

Ok, I've rebased to the current main branch.

droideck
droideck approved these changes Aug 6, 2025
View reviewed changes
Copy link
Contributor

@droideck droideck left a comment

Choose a reason for hiding this comment

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

LGTM!
Thank you!

@droideck droideck merged commit 7af3125 into python-ldap:main Aug 6, 2025
16 checks passed
spaceone added a commit to spaceone/python-ldap that referenced this pull request Aug 8, 2025
@spaceone
feat(ldapobject): allow passing uri=None in SimpleLDAPObject
24218ca 
and `ReconnectLDAPObject` to be consistent with `initialize()`.

Fixes: 7af3125
Issue: python-ldap#465
@spaceone spaceone mentioned this pull request Aug 8, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@droideck droideck droideck approved these changes

@mistotebe mistotebe mistotebe left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@steffen-kiess @mistotebe @droideck @tiran