Skip to content

throw ServerNotAvailableException when LDAP is caught shutting down - #7662

Merged
rullzer merged 1 commit into
masterfrom
ldap-shutdown
Jan 2, 2018
Merged

throw ServerNotAvailableException when LDAP is caught shutting down#7662
rullzer merged 1 commit into
masterfrom
ldap-shutdown

Conversation

@blizzz

Copy link
Copy Markdown
Member

As seen on our instance. Otherwise, processing just continues and who knows what happens.

In general it's probably a better idea to throw exceptions by default and only let successes and other acceptable codes pass. Whitelisting, basically. Would be a separate PR though.

@nextcloud/ldap

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@codecov

codecovBot commented Jan 2, 2018

Copy link
Copy Markdown

Codecov Report

Merging #7662 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@ Coverage Diff @@## master #7662 +/- ##
============================================
- Coverage 51.17% 51.17% -0.01% - Complexity 24885 24886 +1 
============================================
Files 1601 1601 Lines 94746 94749 +3 Branches 1368 1368 ============================================
Hits 48486 48486 - Misses 46260 46263 +3
Impacted FilesCoverage ΔComplexity Δ
apps/user_ldap/lib/LDAP.php16.23% <0%> (-0.43%)52 <0> (+1)

@MorrisJobkeMorrisJobke mentioned this pull request Jan 2, 2018
30 tasks

@rullzerrullzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fine by me.

But yes lets go for whitelisting as well!

@blizzz

Copy link
Copy Markdown
MemberAuthor

But yes lets go for whitelisting as well!

ty. opened a ticket for 14 #7675

@blizzz

Copy link
Copy Markdown
MemberAuthor

backports done ^

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsbugfeature: ldap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@blizzz@rullzer@MorrisJobke