ClusterLabs / OCF-spec

http://standards.clusterlabs.org
20 stars 11 forks source link

Add optional attribute 'required' for parameters/parameter #8

Closed marxsk closed 6 years ago

marxsk commented 6 years ago

This attribute is already heavily used so it should be in standard. It looks very useful but often it would be preferable to have an option to enter more complex queries (e.g enter X or Y)

jnpkrn commented 6 years ago

On 15/12/17 13:01 -0800, Marek Grác wrote:

This attribute is already heavily used so it should be in standard.

No complaints about the intent but rather about the form that I suggest we should rethink prior to introducing any effective changes just following then already established patterns.

Within such initial restructuring, what I'd like to see, we move the textual explanations from the sample schema instances to the schema itself, using annotations, i.e., custom namespaces for that: http://relaxng.org/tutorial-20011203.html#IDA1OZR). This is also what clufter uses to track corosync's configuration space, also by the means of RelaxNG schema: https://pagure.io/clufter/blob/master/f/formats/corosync/corosync.rng

Instances are/will not be very scalable to document the metadata provisions, especially since some items could themselves become mutually exclusive (also per the wish below), and they should be rather self-explanatory, whereas for comprehensive picture, you are visiting the schema anyway.

Doing changes like this from the get-go will save the maintenance burden later on.

It looks very useful but often it would be preferable to have an option to enter more complex queries (e.g enter X or Y)

Colour me embarrassed ... someone's gotta present a demo how inline RelaxNG could work in this scenario:

http://oss.clusterlabs.org/pipermail/users/2016-March/002559.html

Luckily, RelaxNG has a self-expressive power so its grammar can be referenced from within RA API's one (http://relaxng.org/relaxng.rng) :)

-- Poki

marxsk commented 6 years ago

I'm not against the idea of having more complex validation scenarios but I have several objections to it.

2017-12-17 22:36 GMT+01:00 Jan Pokorný notifications@github.com:

On 15/12/17 13:01 -0800, Marek Grác wrote:

This attribute is already heavily used so it should be in standard.

No complaints about the intent but rather about the form that I suggest we should rethink prior to introducing any effective changes just following then already established patterns.

Within such initial restructuring, what I'd like to see, we move the textual explanations from the sample schema instances to the schema itself, using annotations, i.e., custom namespaces for that).

We are not restructuring anything, the current process just focus on getting OCF in sync with resource agents. So that a minimum amount of work is required (e.g. adding an element is fine; making element optional is fine as well) and perhaps removing extensions there were never used. Although (not only) Ken has objections against breaking backward compatibility.

Instances are/will not be very scalable to document the metadata

provisions, especially since some items could themselves become mutually exclusive (also per the wish below), and they should be rather self-explanatory, whereas for comprehensive picture, you are visiting the schema anyway.

Doing changes like this from the get-go will save the maintenance burden later on.

Yes, I agree in general. I see two reasons why to not do it:

It looks very useful but often it would be preferable to have an option to enter more complex queries (e.g enter X or Y)

Colour me embarrassed ... someone's gotta present a demo how inline RelaxNG could work in this scenario:

http://oss.clusterlabs.org/pipermail/users/2016-March/002559.html

Luckily, RelaxNG has a self-expressive power so its grammar can be referenced from within RA API's one (http://relaxng.org/relaxng.rng) :)

Be aware that data that you want to validate are not in XML so you will have to do transform them before any relaxng validation.

jnpkrn commented 6 years ago

On 18/12/17 03:56 -0800, Marek Grác wrote:

2017-12-17 22:36 GMT+01:00 Jan Pokorný notifications@github.com:

On 15/12/17 13:01 -0800, Marek Grác wrote:

This attribute is already heavily used so it should be in standard.

No complaints about the intent but rather about the form that I suggest we should rethink prior to introducing any effective changes just following then already established patterns.

Within such initial restructuring, what I'd like to see, we move the textual explanations from the sample schema instances to the schema itself, using annotations, i.e., custom namespaces for that).

We are not restructuring anything, the current process just focus on getting OCF in sync with resource agents.

This contradicts the fact you've modified directory structure already -- which I follow up on in https://github.com/ClusterLabs/OCF-spec/pull/7.

And if it's not clear, the best timing to settle on structural/form changes is NOW, prior to concentrating on the content itself.

So that a minimum amount of work is required (e.g. adding an element

is fine; making element optional is fine as well) and perhaps removing extensions there were never used.

That's the "content" part of the changes.

Although (not only) Ken has objections against breaking backward compatibility.

Depends on what we want to call OCF 1.0, clearly the roadmap should be:

  1. structural/form changes
  2. do whatever is needed to proclaim OCF 1.0
  3. follow with new/next versions, breaking the compatibility or not, we will have the right measures in place already to cope with both

For that reason, I am reverting "draft" status in said PR #7.

Instances are/will not be very scalable to document the metadata provisions, especially since some items could themselves become mutually exclusive (also per the wish below), and they should be rather self-explanatory, whereas for comprehensive picture, you are visiting the schema anyway.

Doing changes like this from the get-go will save the maintenance burden later on.

Yes, I agree in general. I see two reasons why to not do it:

  • maintainability - in order to use more complex rules, they should be used in agent itself for validation so they cannot be updated. But this is too much work as we have (most of the) agents in bash/python/C. Current setup when the rules are hard-coded and have to be duplicated is clearly sub-optimal but it is easy to code and maintain.

  • those formal rules won't help anyone.

    • If you want to check if validation works, you can call 'validate-all' (later also as machine-readable output)
    • Agent itself won't benefit from them
    • Such rules are not enough for UI without making things even more complex. You will have to add human readable error message (relatively easy), but you have to group those elements together in the UI and make it visible for users. In the picus, we will generate forms dynamically where possile but for some parts, we will have pre-defined subforms (e.g. ssh/password/... or ipv4/ipv6).

I think you are responding to something else than to "move schema documentation from examples/metadata instances to the schema itself" that my previous reply (that you respond to) was addressed at.

You must be rather responding to the idea of metadata instance itself encoding RelaxNG schema snippet to apply further constraints amongst the parameters mentioned at the very same metadata document.

Re maintainability, it should be bearable by the means of:

And, sure UI and other parts can benefit from that change:

It looks very useful but often it would be preferable to have an option to enter more complex queries (e.g enter X or Y)

Colour me embarrassed ... someone's gotta present a demo how inline RelaxNG could work in this scenario:

http://oss.clusterlabs.org/pipermail/users/2016-March/002559.html

Luckily, RelaxNG has a self-expressive power so its grammar can be referenced from within RA API's one (http://relaxng.org/relaxng.rng) :)

Be aware that data that you want to validate are not in XML so you will have to do transform them before any relaxng validation.

I am perfectly aware, don't be afraid.

-- Poki

marxsk commented 6 years ago

2017-12-18 14:35 GMT+01:00 Jan Pokorný notifications@github.com:

On 18/12/17 03:56 -0800, Marek Grác wrote:

Depends on what we want to call OCF 1.0, clearly the roadmap should

be:

  1. structural/form changes
  2. do whatever is needed to proclaim OCF 1.0
  3. follow with new/next versions, breaking the compatibility or not, we will have the right measures in place already to cope with both

It is almost as you said. OCF RA 1.0 is a version obtained from DTD (which was never officialy published) OCF RA 1.1 contains a minor tweaks that are already used in resource agents (without an attempt to cover fence agents)

OCF FA 1.(1 or 2?) = OCF RA 1.1 + fence agent extensions that are not applicable to RA

OCF RA 2.0 - major rework if required that can break backward compatibility

I think you are responding to something else than to "move schema

documentation from examples/metadata instances to the schema itself" that my previous reply (that you respond to) was addressed at.

yup, you are right

You must be rather responding to the idea of metadata instance itself encoding RelaxNG schema snippet to apply further constraints amongst the parameters mentioned at the very same metadata document.

Re maintainability, it should be bearable by the means of:

  • datatype library being part of OCF, so that metadata instances can reference them easily
  • once validate-all is turned to take this extended validation into account, a lot of redundant, procedural code can be dropped in favour of this declarative approach

That's incorrect. You will remove simple if-and-or commands with a declarative approach that will need much more complex code to be parsed (hopefully in the form of the other module). Procedural code will still be used because this approach is not able to react to the real system (e.g. check if private key is a real file or not). It is hard to say how much code could be removed.

In a case of fence-agents' library, we would be able to replace ~15 lines of code (including error messages that will have to be there anyway). In an other case of (random) resource agent apache - we will not be able to remove any line.

Agents are used independently by many different users/applications so it have to take care about input. In addition, as written before validate-all can be done only by agent. So every rule-based system will be able to check just part of the input.

... (agents are stateful, configuration being the state, so effectively I anticipate the "state change" will be the primarly focus of any validations, allowing agents to execute straightforwardly for the particular actions without the need to recheck everything over and over counter-productively)

Agents are stateless, they don't store configuration anywhere. But still, if you have configuration stored in resource manager, the validity of the configuration is dependant on the whole system (e.g. user is able to remove rpm with application, so the file does not longer exists)

And, sure UI and other parts can benefit from that change:

  • validate-all was already mention to benefit from extended validation

validate-all works, so it won't be any benefit

But the 'web form' is not a standard thing, there is not even a set of components. Not only that there are different frameworks, different size of screens but also you have to take care about mobile apps.

kgaillot commented 6 years ago

Merging. The other concerns in this thread are worth discussing, but they are not related to the "required" attribute, so separate PRs would be appropriate.