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

Fixing #15064 - to not fail ldap sync on single data issue with ldap … #15558

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

maciej-poleszczyk
Copy link

Description

Change answers issue described in #15064 and the goal is to not fail whole ldap synchronization because of single invalid manager entry.

Fixes #15064

Type of change

Please delete options that are not relevant.

  • [ x] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Changes were tested with SnipeIT instance that encountered described issue. After introducing change the sync process finishes even if there were some issues with individual entries.

  • Test A
  • Test B

Test Configuration:

  • SnipeIT version: v6.1.1-pre - build 10653 (master)
  • PHP version: 7.x
  • MySQL version: n/a
  • Webserver version: n/a
  • OS version: n/a

Checklist:

  • [ x] I have read the Contributing documentation available here: https://snipe-it.readme.io/docs/contributing-overview
  • [ x] I have formatted this PR according to the project guidelines: https://snipe-it.readme.io/docs/contributing-overview#pull-request-guidelines
  • [x ] My code follows the style guidelines of this project
  • [x ] I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • [n/a] I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • [n/a] I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link

what-the-diff bot commented Sep 25, 2024

PR Summary

  • Improved Error Handling for LDAP Manager Retrieval
    We have taken steps to better manage unexpected situations that arise when trying to fetch information related to the LDAP manager's username and corresponding user. Previously, if the system failed to get the data, it would cause trouble downstream in our software. Now, we handle these exceptions more gracefully which can prevent the system from crashing in the event of failure at this point.

  • Added Conditional Caching for LDAP Manager Data
    We've introduced a new switch that helps us better manage how we store (cache) LDAP manager data. The data is now only cached when the system successfully retrieves it. This improvement means we only store valid data, avoiding the waste of resources on storing unsuccessful or faulty data.

  • Optimized Caching Mechanism
    Further enhancements made to how we cache the LDAP manager's data makes the process more efficient as it only stores the manager's ID when it's appropriate. This decision reduces the storage footprint of our caching system, saving valuable resources.

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.

1 participant