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 issues with connection validation #108

Merged
merged 6 commits into from
Jul 12, 2024
Merged

Conversation

dexter2206
Copy link
Contributor

Description

Currently, we use field_validator for validating if all endpoints in all connections exist. However, the problem with this approach is that it relies on children of a routine being successfully validated prior to validating connections. In a likely event that a child fails to validate, the "children" key will not be present in validation data, and the validator for connections will fail with a TypeError. This PR fixes this by reimplementing connection validator as model_validator that runs iff per-field builtin validators run successfully (otherwise validating connection endpoints is meaningless).

In addition, this PR fixes a hidden problem of incorrect loading of negative testcases for pydantic model validation. Currently, those tests passed trivially (!) because the loaded data consisted of tuples instead of dictionaries.

The documentation has not been updated because the changes are not user-facing, and modified internal parts are not described in the docs.

Please verify that you have completed the following steps

  • I have self-reviewed my code.
  • I have included test cases validating introduced feature/fix.
  • I have updated documentation.

@cla-bot cla-bot bot added the cla-signed label Jul 11, 2024
@dexter2206 dexter2206 changed the title Fix issues with connection verification Fix issues with connection validation Jul 11, 2024
@dexter2206 dexter2206 merged commit 13f0325 into main Jul 12, 2024
7 checks passed
@dexter2206 dexter2206 deleted the fix-verification-issue branch July 12, 2024 11:35
@sitong1011 sitong1011 requested review from sitong1011 and removed request for sitong1011 July 12, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

1 participant