[Freeipa-devel] [PATCH] Password vault

[Endi Sukma Dewata]

On 6/3/2015 8:52 AM, Alexander Bokovoy wrote:
>> Having to use the same plugins for client and server is a framework
>> limitation/poor design. Having to use conditional imports to work
>> around the limitation is a bad programming practice. The fact that
>> trust plugin has to implement a similar workaround is not a
>> justification, it just shows that the problem is not vault-specific.

> There is another thing. Even when splitting client/server sides, we'll
> need to check on the server side that certain functionality is
> available. In trust case we have ID Views (a separate plugin) which does
> use information about trusts to resolve users from AD to their
> normalized references (SIDs) and few other places would be depending on
> functionality only provided when Samba packages are installed.
>
> To continue your approach, we would need to split also server-side parts
> of plugins into separate callable units that would only be provided and
> called when appropriate rpm subpackages are installed. This is unneeded
> complication in place where we can simply handle dependencies in run
> time and make sure the packaging deps are managed separately.

So there are two issues:
1. Conditional imports due to combined client and server plugin.
2. Conditional imports due to feature availability.

Issue #1 is generic and I think we pretty much agree that this is 
supposed to be fixed somehow.

Issue #2 is plugin-specific. I think there are different ways to solve 
this, some might be better than others. The solution that you pick will 
only affect that particular plugin and should not be generalized to 
other plugins or to justify #1.

In my opinion a code should have a fixed dependency, but some features 
can be enabled/disabled based on the configuration. Enabling a feature 
shouldn't be based on package installation because people might install 
a package for different reasons. So the code may look something like this:

   import <module>

   if <feature is enabled>:
       <do something with the module>

It shouldn't be like this:

   if <package is installed>:
       import <module>

   if <package is installed>:
       <do something with the module>

Of course this assumes that the package is lightweight enough to be 
installed regardless whether it will be used. I don't know if it's 
applicable to your case, but again, there are different ways to address 
issue #2.

-- 
Endi S. Dewata