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

Feature/validators - MLTE-507, MLTE-150 #554

Open
wants to merge 48 commits into
base: master
Choose a base branch
from

Conversation

sei-secheverria
Copy link
Contributor

@sei-secheverria sei-secheverria commented Dec 14, 2024

Restructured Conditions to have a Validator inside that can guide the creation of the Condition, with specific arguments for bool expression, success message, etc, instead of depending on user of build_condition() figuring out the structure of the whole callback.

More specifically:

  • Addresses Guide Creation of Condition Validation #507 by adding a Validator inside a Condition, and moving boilerplate code for validation inside Validator, leaving just a bool_exp as the lambda to be passed, separating success, failure, and info messages into own args
  • Addresses Handle non-serializable Condition arguments when converting to/from JSON #150 by adding json_fix and loading it into internal wrapper json library to be used, which allows any class to implement the __json__() method and allow json.dumps to work when serializing it (it was applied to Artifact), but in theory people could add it to their own complex Condition arguments if needed).
  • Also addresses Handle non-serializable Condition arguments when converting to/from JSON #150 by centralizing how JSON is created into a specific method in BaseModel, and creating specific exception for serialization errors
  • Partially addresses Create Condition/Validator without creating/extending a type #508 by letting non-validator args of Condition be optional, in theory allowing the creation of Conditions directly without calling build_condition, but this is not clearly shown or explained in examples, nor fully tested in the unit tests yet, so it is mostly hidden. Functionality will be reviewed and clarified when Create Condition/Validator without creating/extending a type #508 is fully addressed in another PR.
  • Renamed the "Ignore" result type into "Info", to more explicitly indicate that it is used to attach additional info, instead of indicating when it is used.
  • Added missing unit tests for methods that build conditions
  • Added sample image to be used in unit tests for Image class, instead of downloading image from the Internet, removing that dependency

…erate a standard and more informative exception if artifact or other model can't be serialized.
…ternal ConditionModel function, to better handle errors there.
…hon 3 is no longer needed separately from __eq__
…ide it. For DB serialization, Validator is stored as a JSON string, at least for now.
…s where we want to allow conditions to be created outside of a Value class.
…added checks to see that combination of None and existing args are valid
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

Successfully merging this pull request may close these issues.

2 participants