I don't think this is correct. Attribute Unique is not based under a single cn like cn=attribute uniqueness with configs, it's multiple instances of the plugin, and they can be anywhere under cn=plugins. So this container may not exist (which could cause plugin create to fail), but also that it means instances of this plugin may not be found.

This in mind, attribute unique is absolutely cursed because of the way that it works like this, and it totally breaks down in the model that is lib389, and compounded by extensibleObject ....

So sadly, I think in this case, we can't make this change here to plugins.py, and if people do have a case where it's not finding the plugin, we'll need to work that out case by case .... :( :(

If I understand correctly, you do this because of the progress bar. Am I right?
I am just a bit concerned that you are deprecating this code... If I understand correctly, it will still show the create instance button but it will fail to create it with some obscure message.
I think we should deal with it somehow...

This looks a bit strange... You remove the loading toggler from managedEntries->loadConfigs, from attributeUniqueness but you don't remove it from some other places (i.e. linkedAttributes)
Also, I don't see the BZ it refers to in the commit message. Do I miss something?..

If you remove this, then it won't update the plugin list when we switch between the instances

If you remove this, then it won't update the plugin list when we switch between the instances

When we switch between instances EVERYTHING is reloaded, no?

This looks a bit strange... You remove the loading toggler from managedEntries->loadConfigs, from attributeUniqueness but you don't remove it from some other places (i.e. linkedAttributes)
Also, I don't see the BZ it refers to in the commit message. Do I miss something?..

I said in the commit that I improved the plugin loading, that's what this change is part of. I removed the toggling around the loading because it was redundant. We already do the toggling when we add/save or delete a managed entry config. Otherwise we toggle like 3 times on the managed entry config and the UI looks really sloppy as we are constantly spinning, then stops spinning, spins again, stops spinning, etc. It does it 3 times - well it used to that is.

This looks a bit strange... You remove the loading toggler from managedEntries->loadConfigs, from attributeUniqueness but you don't remove it from some other places (i.e. linkedAttributes)
Also, I don't see the BZ it refers to in the commit message. Do I miss something?..

I said in the commit that I improved the plugin loading, that's what this change is part of. I removed the toggling around the loading because it was redundant. We already do the toggling when we add/save or delete a managed entry config. Otherwise we toggle like 3 times on the managed entry config and the UI looks really sloppy as we are constantly spinning, then stops spinning, spins again, stops spinning, etc. It does it 3 times - well it used to that is.

Oh, and linked attributes does not exhibit this behaviour, it was only managed entries that did all this excessive toggling.

If I understand correctly, you do this because of the progress bar. Am I right?
I am just a bit concerned that you are deprecating this code... If I understand correctly, it will still show the create instance button but it will fail to create it with some obscure message.
I think we should deal with it somehow...

Ok I need to test the create instance button, but you can not have cockpit-389-ds without the 389-ds-base package. It's a hard dependency so there is no reason to check for it. Also people who build the products from source, and people on other platforms (debian for example) run into problems with the rpm check. Since it's not needed I removed it.

I don't think this is correct. Attribute Unique is not based under a single cn like cn=attribute uniqueness with configs, it's multiple instances of the plugin, and they can be anywhere under cn=plugins. So this container may not exist (which could cause plugin create to fail), but also that it means instances of this plugin may not be found.
This in mind, attribute unique is absolutely cursed because of the way that it works like this, and it totally breaks down in the model that is lib389, and compounded by extensibleObject ....
So sadly, I think in this case, we can't make this change here to plugins.py, and if people do have a case where it's not finding the plugin, we'll need to work that out case by case .... :( :(

Well currently dsconf does not work with attribute uniqueness at all if any of the plugin entries are disabled, because when its disabled all the plugin specific attributes, like vendor, funcs, ID's, etc are all none. This is how lib389 was finding the attr uniq plugin entries, so when its "off" it becomes invisible and there is no way to enable it.

Maybe we should enforce it being under a container? Regardless you are right, currently it's broken with or without my fix. I'll look into it some more...

When we switch between instances EVERYTHING is reloaded, no?

React reacts on the change of state. Do the child components - the only proper way now is manually define when we should react. Basically, the current structure (dsconf calls stuff) is the only explicit and simple way to do this.
After we will create one batch dsconf call which loads everything we can get rid of these things. But now... It's a lesser evil.

I said in the commit that I improved the plugin loading, that's what this change is part of. I removed the toggling around the loading because it was redundant. We already do the toggling when we add/save or delete a managed entry config. Otherwise we toggle like 3 times on the managed entry config and the UI looks really sloppy as we are constantly spinning, then stops spinning, spins again, stops spinning, etc. It does it 3 times - well it used to that is.

Sure, it makes sense and I agree. :)
My point is that it is inconsistent now. Other plugin components have the same toggling thingy.

When we switch between instances EVERYTHING is reloaded, no?

React reacts on the change of state. Do the child components - the only proper way now is manually define when we should react. Basically, the current structure (dsconf calls stuff) is the only explicit and simple way to do this.
After we will create one batch dsconf call which loads everything we can get rid of these things. But now... It's a lesser evil.

Previously when I switch instances I "thought" it reloaded everything but I did not confirm it. Anyway I will gladly add that code back...

Ok I need to test the create instance button, but you can not have cockpit-389-ds without the 389-ds-base package. It's a hard dependency so there is no reason to check for it. Also people who build the products from source, and people on other platforms (debian for example) run into problems with the rpm check. Since it's not needed I removed it.

I just tried to remove 389-ds-base and it hasn't removed cockpit-389-ds. So we probably should make it harder at that point.
But otherwise, I am okay with the change

I said in the commit that I improved the plugin loading, that's what this change is part of. I removed the toggling around the loading because it was redundant. We already do the toggling when we add/save or delete a managed entry config. Otherwise we toggle like 3 times on the managed entry config and the UI looks really sloppy as we are constantly spinning, then stops spinning, spins again, stops spinning, etc. It does it 3 times - well it used to that is.

Sure, it makes sense and I agree. :)
My point is that it is inconsistent now. Other plugin components have the same toggling thingy.

What I saw was that when we called pluginList somehow managed entry config loading was toggling three times. No other plugin did this, but I'll take a look at the other plugins like linked attributes and revise them if needed.

Ok I need to test the create instance button, but you can not have cockpit-389-ds without the 389-ds-base package. It's a hard dependency so there is no reason to check for it. Also people who build the products from source, and people on other platforms (debian for example) run into problems with the rpm check. Since it's not needed I removed it.

I just tried to remove 389-ds-base and it hasn't removed cockpit-389-ds. So we probably should make it harder at that point.
But otherwise, I am okay with the change

Hmm, you are right but I recall it being a dependency in the past. Perhaps the upstream (or source) spec file is off? I'll look into that...

Okay @spichugi and @firstyear "everything" is fixed, please review...

Looks good to me for the parts I've reported and I haven't found anything new, so...
Ack from me!
I'll wait for @firstyear to confirm too :)

I don't think this is correct. Attribute Unique is not based under a single cn like cn=attribute uniqueness with configs, it's multiple instances of the plugin, and they can be anywhere under cn=plugins. So this container may not exist (which could cause plugin create to fail), but also that it means instances of this plugin may not be found.
This in mind, attribute unique is absolutely cursed because of the way that it works like this, and it totally breaks down in the model that is lib389, and compounded by extensibleObject ....
So sadly, I think in this case, we can't make this change here to plugins.py, and if people do have a case where it's not finding the plugin, we'll need to work that out case by case .... :( :(

Well currently dsconf does not work with attribute uniqueness at all if any of the plugin entries are disabled, because when its disabled all the plugin specific attributes, like vendor, funcs, ID's, etc are all none. This is how lib389 was finding the attr uniq plugin entries, so when its "off" it becomes invisible and there is no way to enable it.
Maybe we should enforce it being under a container? Regardless you are right, currently it's broken with or without my fix. I'll look into it some more...

Honestly that whole plugin just needs it's configuration syntax fixed, and until then lib389 will always struggle to manage it :(

This is absolutely horrible, but I'm also sure it works, so you know what, yolo.

Ack from my side re the plugins/python bits that I checked :)

This is absolutely horrible, but I'm also sure it works, so you know what, yolo.

Oh I know. At least the plugin function attribute is static and is not set whether the plugin is enabled or not. Lesser of two evils until we can rework the plugin. That's more of a 1.4.4 or 1.5.0 change...

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/4048

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.