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

Check with regex.match() evaluated before "required attribute missing" #1337

Closed
excalq opened this issue May 20, 2024 · 3 comments · Fixed by #1341
Closed

Check with regex.match() evaluated before "required attribute missing" #1337

excalq opened this issue May 20, 2024 · 3 comments · Fixed by #1341
Labels
enhancement New feature or request evaluator runtime Issues or PRs related to kcl runtime including value and value opertions

Comments

@excalq
Copy link

excalq commented May 20, 2024

Bug Report

When a schema contains a check, using regex.match(attribute ...), this appears to be evaluated before the existence of a non-optional attribute in the instantiated object.

If the required attribute is absent, a compiler error is thrown regarding the schema's check regex.match(attribute) itself, rather than notification that the user's object is lacking a required attribute. The more obvious error would be raising that the attribute is missing (attribute .. is required and can't be None or Undefined).

As it were, the current error causes a user to debug a problem with the check. In my case the missing field was in an inherited schema's object, compounding my confusion.

The reproduction will easily illustrate this.

1. Minimal reproduce step (Required)

Playground Link

import regex

schema CrossplaneResource:
  name: str
  description: str
  check:
    regex.match(description, "[a-zA-Z0-9+=,.@-_]"), "Description must match the regex [a-zA-Z0-9+=,.@-_]"

ice: CrossplaneResource = {
  name: 'Vanilla Ice'
  # description: 'Ice Ice Baby' # Works with this field uncommented
}

2. What did you expect to see? (Required)

The missing field should fail with an error before the check dies on the missing description variable.

EvaluationError
  --> /tmp/sandbox1241309759/prog.k:15:1
   |
15 |   name: 'Vanilla Ice'
   |  attribute 'description' of CrossplaneResource is required and can't be None or Undefined
   |

3. What did you see instead (Required)

EvaluationError
  --> /tmp/sandbox1225202080/prog.k:11:1
   |
11 |     regex.match(description, "[a-zA-Z0-9+=,.@-_]"), "Description must match the regex [a-zA-Z0-9+=,.@-_]"
   |  match() missing 2 required positional arguments: 'string' and 'pattern'
   |

4. What is your KCL components version? (Required)

0.8.7-darwin-arm64

@excalq
Copy link
Author

excalq commented May 20, 2024

Another aspect to this is being unsure if it's possible to do validation on optional fields (if set). I assumed I could do:

check:
    description == None or regex.match(description, ...), 'The error message'

But this produces the same error reported above. My workaround is having the field be non-optional, and setting a default description value in the schema.

@Peefy Peefy added enhancement New feature or request evaluator labels May 21, 2024
@Peefy Peefy added this to the v0.9.0 Release milestone May 21, 2024
@Peefy
Copy link
Contributor

Peefy commented May 21, 2024

You can write the following code for the optional attribute using the if filter, and I will change the optional attribute check time in some PR.

check:
     regex.match(description, ...) if description, 'The error message'

@excalq
Copy link
Author

excalq commented May 21, 2024

Well that makes a lot of sense, thinking Pythonically! Thanks a ton.

@Peefy Peefy added the runtime Issues or PRs related to kcl runtime including value and value opertions label May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request evaluator runtime Issues or PRs related to kcl runtime including value and value opertions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants