LDAP refactoring: remove admin_conn #223
Conversation
tkrizek
force-pushed
the
refactoring-ldap4
branch
from
47adf27
to
cb181fd
Compare
|
Bump for review. I forgot about this PR, it's a final change from the refactoring effort. |
|
Works for me, I'll check code tomorrow |
|
ACK: removing admin_conn connections
NACK: upgrade commit
|
Since service.admin_conn is only an alias to api.Backend.ldap2, replace it everywhere with the explicit api.Backend.ldap2 instead. https://fedorahosted.org/freeipa/ticket/6461
tkrizek
force-pushed
the
refactoring-ldap4
branch
from
cb181fd
to
fd19f40
Compare
|
Fixed the issue + rebased. Only the second commit has changed. |
| ds.fqdn = fqdn | ||
| ds.realm = api.env.realm | ||
| ds.suffix = ipautil.realm_to_suffix(api.env.realm) | ||
| ds.principal = "ldap/%s@%s" % (ds.fqdn, ds.realm) |
There was a problem hiding this comment.
Discovered this after rebase, this was probably a regression from installer refactoring. principal is now a property.
There was a problem hiding this comment.
Please write this to installer refactoring ticket
There was a problem hiding this comment.
OR remove it in separate patch
tkrizek
force-pushed
the
refactoring-ldap4
branch
2 times, most recently
from
4bff799
to
89bd31a
Compare
|
Depends on #262 |
| if ds_running and not ds.is_running(): | ||
| ds.start(ds_serverid) | ||
| elif not ds_running and ds.is_running(): | ||
| if not ds_running: |
There was a problem hiding this comment.
I see you stopped removed many ds.start() lines. I'm afraid that some of them may be missing in case there is a function that stops DS. Did you make sure that this cannot happen?
There was a problem hiding this comment.
I think these were relics, since I've not been able to find a use case in which they'd be necessary.
|
LGTM and Works for me, but I have to make sure that things I wrote inline won''t happen |
Clean up unnecessary starts/stops of DS and unnescessary attributes. If the DS is running, establish an LDAP connection and properly close it. https://fedorahosted.org/freeipa/ticket/6461
tkrizek
force-pushed
the
refactoring-ldap4
branch
from
89bd31a
to
355cc51
Compare
This first commit removes the admin_conn alias for api.Backend.ldap2 that was previously used in services.
When trying to get rid of it, I found some legacy code in ipa-server-upgrade. The second commit improves ldap connection management in upgrade and removes useless start and stops of directory server at random places.