Skip to content

Conversation

@adamluzsi
Copy link
Contributor

@adamluzsi adamluzsi commented Sep 9, 2025

When an LDIF file exceeds memory limits, processing it entry‑by‑entry will help bypass those constraints.

Changes:

  • Unmarshal logic got refactored to not collect the entries in the LDIF.Entries field but rather yield back,
  • For backwards compatibility, the change is exposed in the package as UnmarshalEntries, and the Unmarshal public behaviour remained untouched.
  • New behaviour is documented through extended test coverage

When an LDIF file exceeds memory limits,
processing it entry‑by‑entry will help bypass those constraints.
@adamluzsi
Copy link
Contributor Author

@vetinari @liggitt

@adamluzsi
Copy link
Contributor Author

@johnweldon @cpuschma

Could you give me a rough estimate on how long it would take to review the change?

@cpuschma cpuschma self-assigned this Sep 10, 2025
@cpuschma
Copy link
Member

LGTM. If no one objects I'll merge the PR today. Thank you 👍

@adamluzsi
Copy link
Contributor Author

Could you please check the changes again?
I missed noticing your earlier message and pushed a small change that replaces the deprecated ioutil references with the officially recommended os package variants. Same behaviour, just different package import

If this causes any problems as part of the PR, I can revert the change.

@adamluzsi
Copy link
Contributor Author

Additionally, it is worth mentioning that as part of this PR, I updated the Go version in the go.mod file to use a Go version that's only two releases behind the current stable release (1.23, while the latest is 1.25).

@cpuschma
Copy link
Member

Could you please check the changes again? I missed noticing your earlier message and pushed a small change that replaces the deprecated ioutil references with the officially recommended os package variants. Same behaviour, just different package import

If this causes any problems as part of the PR, I can revert the change.

This is fine and shouldn't cause any issues.

Additionally, it is worth mentioning that as part of this PR, I updated the Go version in the go.mod file to use a Go version that's only two releases behind the current stable release (1.23, while the latest is 1.25).

That's fine, it's necessary to access the new iter package anyways.

@cpuschma cpuschma merged commit aa3bc30 into go-ldap:master Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants