Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should 'constraints' keyword be renamed? #136

Open
lauwers opened this issue Nov 29, 2022 · 9 comments
Open

Should 'constraints' keyword be renamed? #136

lauwers opened this issue Nov 29, 2022 · 9 comments

Comments

@lauwers
Copy link
Contributor

lauwers commented Nov 29, 2022

In Tal's latest proposal, the 'constraints' keyword has been renamed to 'validation'. The meaning of the 'validation' keyword is less intuitive than the original 'constraints', but the 'constraints' keyword suggest a multitude of constraints, which is no longer accurate now that we have introduced function syntax for expressing constraints/validations. Is there a better alternative or should we stick with the original 'constraints'?

@pmjordan
Copy link
Contributor

A keyword 'validation' with a boolean type ought to indicate that validation is/is not applied; but that is not what is intended here. In the proposal whether validation is applied or not is controlled by the prescence or abscence of the key word.
I belive it would be more accurate to make the new keyword 'valid_if'; i.e the parameter is only valid iff (sic) teh function evaluates to True.

@tliron
Copy link
Contributor

tliron commented Nov 30, 2022

Here are a bunch of ideas and thoughts:

  • constraints: The idea of keeping it the same is clearly beneficial, but it is just no longer correct in my view. First, in that the plural form is no longer reflective of the value. Second, in that validity is a more general concept. For example, validity for an attribute could depend on external (runtime) system state. It's true that in a very general sense any kind of test for validity "constrains" a value, but to me that is not the primary function here.
  • valid: Nice and short, but perhaps not entirely clear.
  • valid_if: Technically the best, but it doesn't quite fit with the way other TOSCA keywords are structured, as it's a sentence fragment in English. I feel that if we do this here there are probably a few other TOSCA keywords that could also benefit from this style.
  • valid_when: Same. Perhaps not as good as "if", because it raises the question of "when" the validity check happens. We assume it's when the value is "put into" the box that holds it (property, attribute, column in db, etc.).
  • is_valid: This variant is not a sentence fragment, so may fit better with other TOSCA keywords.

@pmjordan
Copy link
Contributor

Even if we did adopt valid_if, the case for revising other keywords to match this style is weak as we are not changing the meaning of those other words.
'is_valid' addresses my concern and I would support that suggestion. However any keyword which includes the word 'valid' does clash somewhat with the existing function name 'valid_values'. There may be case for renaming that to something like 'possible_values'.

@tliron
Copy link
Contributor

tliron commented Nov 30, 2022

For what it's worth, I think $valid_values is a poorly named function... I forget, did we decide to change that due to Calin's suggestions or keep it as is?

@lauwers
Copy link
Contributor Author

lauwers commented Nov 30, 2022

We blindly converted all the v1.x constraint operators into v2.0 built-in functions. We haven't yet had the discussion about whether we should also change the names in the process.

@lauwers
Copy link
Contributor Author

lauwers commented Dec 1, 2022

satisfies ?

@lauwers
Copy link
Contributor Author

lauwers commented Dec 2, 2022

We could also borrow from YANG, and use the keyword must

@pmjordan
Copy link
Contributor

pmjordan commented Dec 2, 2022

I'm not keen on satisfies. To me it says that the value of the parameter always satisfies the constraint function.

@lauwers
Copy link
Contributor Author

lauwers commented Dec 2, 2022

@pmjordan I agree. Technically it should be must_satisfy. But since that is rather long, I would vote for the shorter must

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants