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

fix(ui): Update broken yup-ast schema #81

Conversation

deadlycoconuts
Copy link
Contributor

@deadlycoconuts deadlycoconuts commented Aug 2, 2024

What this PR does / why we need it:
In caraml-dev/turing#390, the yup-ast package has been updated to use a much newer and updated alternative. However, after fixing that problem, it was found that the existing experiment engine yup-ast schema specified in XP isn't working, i.e. the validation schema is invalid and yup allows the entire config to pass even if it fails certain fields in the validation schema.

Apparently what's needed to be fix is to simply add additional square brackets when defining a list of validation checks in the schema specified for a specific field (only when using the yup.shape check). I've tested these changes locally.

I can't tell if this wasn't working in the past or if the schema change is only expected by the newer @demvsystems/yup-ast package so I'm opening both PRs to fix this issue once and for all. (Yes I know that's a TODO to clean this part up but I'm ignoring it for now to get this fix released as soon as possible since it's blocking some users from deploying their Turing routers).

Which issue(s) this PR fixes:

Fixes #

@deadlycoconuts deadlycoconuts added the bug Something isn't working label Aug 2, 2024
@deadlycoconuts deadlycoconuts self-assigned this Aug 2, 2024
Copy link

@tiopramayudi tiopramayudi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ! Thanks @deadlycoconuts

@deadlycoconuts
Copy link
Contributor Author

Thanks a lot for the quick review @tiopramayudi ! Merging this now!

@deadlycoconuts deadlycoconuts merged commit 6ef47af into caraml-dev:main Aug 5, 2024
4 checks passed
@deadlycoconuts deadlycoconuts deleted the fix_xp_experiment_config_schema branch August 5, 2024 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants