> On 17 Mar 2020, at 02:49, thierry bordaz <[email protected]> wrote:
> 
> Hi,
> 
> As a follow up of the PR https://pagure.io/389-ds-base/pull-request/50939,
> I wrote down a small design about  rewriters (filter/computed_attr) plugin: 
> http://www.port389.org/docs/389ds/design/search_rewriters.html
> 
> Comments are welcome

Probably the most dangerous thing to say in all of history?

Like, your design is very smart, but that cleverness and flexibility carries 
many risks. The problem at hand is rewriting ad attributes - not to make a 
framework. I still say focus on that problem alone rather than trying to solve 
a generic class of problems.

Anyway, I still don't think this is the right avenue. There are two major 
reasons for this:

First, is the attempt to make a "generic framework" to solve a "specific 
problem". We should not have a generic rewrite framework, when all we need is a 
specific, focused, module just for doing known and well tested attribute 
transformations. 

Code like COS or MEP may be generic, and it solves many cases but the surface 
area is huge, it's hard to test, and it's hard to reason about. 

We do not have a need for allowing generic, and arbitrary rewriters to exist, 
especially not when you have to "compile in" the rewriters anyway!

This should be simply, an "ad rewrite" plugin, where all it does is that one 
thing - rewrite the attributes as required for AD emulation for IPA. This is 
far easier to deploy, test and reason about. Ideally, the configuration is 
simply "the plugin is enabled or disabled".


Second, is the idea of this being a "search rewriter". I don't think this is a 
good idea. The search path should be simple, it's our hot path. We have many 
things that have to interact like indexes etc. Look at virtual attribute 
indexing and such and the work needed for COS to have these used? 

This plugin should be on the write path, transforming when a change occurs. 
This means the code is much simpler, easier to test, and we need no 
modifications to our read paths. Things like MEP and replication will "just 
work" as will indexing and much more.


For me to approve this plugin, I really want to see it being a write-path 
transformation of values into other values, and it should be focused, targeted, 
and simple. 

I do want to make one thing clear though - I think it's much better that this 
plugin exist in 389-ds rather than in freeipa. The 389-ds project has better 
tooling (like ASAN/LSAN), faster testing capability and a group of subject 
matter experts for code review. I think that if you were to move this to 
freeipa, you would not have the same level of testing or review quality as 
here, so I'd prefer to see you put it here. Sure, I might be difficult on this 
topic, but I do it because I believe there is a better, more robust manner to 
approach this problem space than currently you are considering. :) 


Thanks,


> 
> best regards
> thierry
> _______________________________________________
> 389-devel mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
> Fedora Code of Conduct: 
> https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: 
> https://lists.fedoraproject.org/archives/list/[email protected]

—
Sincerely,

William Brown

Senior Software Engineer, 389 Directory Server
SUSE Labs
_______________________________________________
389-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/[email protected]

Reply via email to