pcs
pcs copied to clipboard
ClusterLabs
Confusing error message in 'pcs constraint ticket add' when an unknown option is specified
A simple way to fix rhbz2022748 : https://bugzilla.redhat.com/show_bug.cgi?id=2022748 perhaps it's better to split 'options' into arguments and options?
Thank you for your patch. We are not going to accept it, however, for the following reasons:
pcs.lib.cib.constraint.constraint._validate_attrib_names is a generic function. Even if it is not used for other than ticket constraints now, it may be used for them later. As such, it should not have specific knowledge about one type of constraints which doesn't apply to other types.
By putting positional_arguments in pcs.lib.cib.constraint.constraint._validate_attrib_namesfunction, you put a knowledge about a client to the pcs library. This is a problem, as the library should be client-independent. What if someone writes a client where rsc, rsc-role and ticket are not positional arguments? How does the changed validation work with web UI?
The correct way to fix this issue is to modify error reports in the client, which in this case is pcs.cli.constraint_ticket.command.add.
I move the _validate_attrib_names func to pcs.cli.constraint_ticket.command.add, is this what you mean?
Sorry for the delay.
I add a testcase this time , but this commit cause pcs_test.tier0.cli.constraint_ticket.test_command.AddTest.test_put_resource_role_to_options_for_library to fail which confused me, wasn't it supposed to be cover by test_refuse_resource_role_in_options?
Should I modify pcs_test/tier0/lib/cib/test_constraint_ticket.PrepareOptionsPlainTest.test_refuse_unknown_attributes as well?
I'm sorry it took us so long to get back to this PR.
It turned out that fixing this issue was not easy and straightforward at all due to suboptimal architecture in constraints creation. Once I figured out what needs to be done, it seemed too time consuming and complex to explain in comparison to writing the patch.
Nevertheless, thank you for your effort.