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

Don't update LDAP if there are nothing to do #231

Open
wants to merge 2 commits into
base: stretch-unstable
from

Conversation

@Josue-T
Copy link
Contributor

Josue-T commented Dec 22, 2019

Problem

If we try to update the LDAP database with no chang slapd crash, and it's a big problem.

Solution

Check if there are a real change to do and if not do nothing.

@Josue-T Josue-T requested review from alexAubin and YunoHost/core-dev Dec 22, 2019
@Josue-T

This comment has been minimized.

Copy link
Contributor Author

Josue-T commented Dec 30, 2019

Well it's quite important to merge this also in the actual testing because some unit tests are broken without this PR (because some empty update has been integrated in the permission factorisation).

@Josue-T Josue-T mentioned this pull request Dec 30, 2019
0 of 4 tasks complete
@kay0u
kay0u approved these changes Jan 14, 2020
Copy link
Member

kay0u left a comment

LGTM, but my slapd don't crash on an empty request... Idk why!

@Josue-T

This comment has been minimized.

Copy link
Contributor Author

Josue-T commented Jan 14, 2020

LGTM, but I my slapd don't crash on an empty request... Idk why!

But on the testing or stable. I think this crash was introduced by some overlay enabled in the testing.

@kay0u

This comment has been minimized.

Copy link
Member

kay0u commented Jan 14, 2020

Yeah, saw it somewhere but I can't reproduce it.

@Josue-T

This comment has been minimized.

Copy link
Contributor Author

Josue-T commented Jan 14, 2020

Actually some unit test are broken following the permission code factorisation. This PR fix this. As I remember it's on the backup/restore part but I don't remember exactly where.

@kay0u

This comment has been minimized.

Copy link
Member

kay0u commented Jan 18, 2020

ok, works for me

@kay0u kay0u added this to the 3.7.x milestone Jan 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.