DIRAC icon indicating copy to clipboard operation
DIRAC copied to clipboard

feat: verify CS config commit using diracx model validation

Open natthan-pigoux opened this issue 6 months ago • 7 comments

Verify the CS config using diracx model validation before committing. If validation fails a warning is diplayed.

closes #8302

natthan-pigoux avatar Oct 23 '25 13:10 natthan-pigoux

I'm not completely sure the verification was expected to be in Modifcator. Also not sure where to write the diracx related code.

natthan-pigoux avatar Oct 23 '25 13:10 natthan-pigoux

I think your understanding is correct, the check should be at modification time.

I would not create a new module, you can add your new function inside Modificator.py or inside src/DIRAC/FrameworkSystem/Utilities/diracx.py

fstagni avatar Oct 23 '25 15:10 fstagni

Thanks @fstagni for the clarification.

natthan-pigoux avatar Oct 24 '25 07:10 natthan-pigoux

One possible issue I see in what I've done is that, if my understanding is correct, in Modificatior, cfgData corresponds to the entire CS config. If there is many validation errors, each time there is a commit all of these are displayed and not only the ones related to the changes.

Is that ok like this? or should I verify only the current changes?

natthan-pigoux avatar Oct 24 '25 07:10 natthan-pigoux

It's probably OK. I am also wondering if instead of simply displaying a warning, the commit should simply be refused.

fstagni avatar Oct 24 '25 08:10 fstagni

It's probably OK. I am also wondering if instead of simply displaying a warning, the commit should simply be refused.

In the issue it is precised that:

Not a failure, because we do not want to prevent the commit from happening

natthan-pigoux avatar Oct 24 '25 08:10 natthan-pigoux

It's probably OK. I am also wondering if instead of simply displaying a warning, the commit should simply be refused.

Here me and @chaen disagree but I also think it should be refused. Maybe it would be good to gate it behind an option in the CS service.

chrisburr avatar Oct 24 '25 08:10 chrisburr

Hi @fstagni , do I need to do or change something more here?

natthan-pigoux avatar Dec 08 '25 09:12 natthan-pigoux

No, I just opened/closed to get the tests running again. I will now hotfix this one and probably merge.

fstagni avatar Dec 08 '25 09:12 fstagni

I hotfixed this one, and tried it out. It works OK.

fstagni avatar Dec 08 '25 09:12 fstagni