Possible improvement for eldap, before opening a PR?

Questions / Help
,
1

Well,

I was headed to open a new pr on github for a small improvement on the eldap app, and discuss it there, but now the guidelines says to discuss such thing in the forum, and since I’m not 100% sure that the proposed change is clean enough, I don’t know what to do :slight_smile:

The patch itself is very small, but adds a new field to a record which is returned back to the user, so may qualify as a breaking change?

Basically, right now on eldap is not possible to perform paginated searches, well is partially possible since on a search ldap additional Controls may be passed right now. But to make all work is needed to pass back Controls sent in the resulting search which contains the “cursor” (cookie in ldap parlance) to be sent again for the next page and so on.

My question is what is better between:

  • exposing the result Controls and leave to the eldap user build subsequent requests: this is a minimal change but alters the record returned to the caller. The caller is more complex, but well, the current apis are not very high level.
  • add a new api to the eldap module, which is a duplication of the search + pagination handling: this may be a bigger patch, but does not change the current interface at all, and makes things simpler for the caller.

I’ve implemented the first (also because imho makes no sense to allow passing ldap controls from callers and not having a way to retrieve response controls back), but need to understand if is enough for opening a PR.

4 Likes
2

Hi @xadhoom and welcome in Erlang Forum :metal:!
First of all I think you create in correct place but tag should use as Questions / Help or please change title like Improvement of eldap. The next important think what need to do I suppose is - ping Erlang Core Team members which you can find here Erlang core team - Erlang Forums. Feel free to point people or mention them in topics :upside_down_face:.

2 Likes
3

ping @hansn

3 Likes
4

Thanks @vkatsuba I’ve updated the topic/tags as you suggested.

An additional scenario that can be resolved exposing search controls in results is sorting of data, which can be easily handled by the caller of the search api and does not require further modification in my first suggested solution.

3 Likes
5

I am confused, There is a PR already PR-5538 which implements the first of your bullets. Is that your PR?

Anyway, I don’t think it makes sense to divide this in two PRs. As far as I understand there must be a way to ask for pagination in the client otherwise there will be no response of the new type.

When it comes to API I think this could be handled in 2 ways.

  • Alt 1: Introduce additional options to the search/2 function. I think it should be ok to return a new type of result only if the new option(s) has been used.
  • Alt 2: Introduce a new function search_paginated with appropriate arguments. The arguments should be designed so that it is easy to iterate over the result pages until all are received (The parameters in the search are changed during the iterations and values received in the SearchResultDone are used in the subsequent calls.

It would also be interesting to see an example with code using the search with pagination.

I also think that in this case we can continue the detailed discussion in the PR.

3 Likes
6

Oh, well…

I totally missed that and we ended up doing the same thing, I’ll add my notes on the PR so :slight_smile:

thanks for pointing out!

2 Likes