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.

Sign up for GitHub

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

Jump to bottom

ADR0007: How we are going to validate the new API codebase #1301

Closed
wants to merge 12 commits into from
Closed

ADR0007: How we are going to validate the new API codebase #1301

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4b2f28c
ADR0007: How we are going to validate the new API codebase
MVrachev Mar 10, 2021
6627853
Correct myself about strict mode in pydantic
MVrachev Mar 15, 2021
5734797
Add 2 requirements and summarize options stances
MVrachev Mar 17, 2021
c6c706c
Document marshmallow library as an option
MVrachev Mar 17, 2021
08853c7
Document invoking validate_model() for pydantic
MVrachev Mar 18, 2021
9c65b3d
Remove typical library as an option
MVrachev Mar 18, 2021
e794539
Close code block for an example
MVrachev Mar 18, 2021
05c36a3
Use one table to compare all options
MVrachev Mar 22, 2021
ca70f30
Document hybrid option of typeguard + decorators
MVrachev Mar 24, 2021
b9e838a
Document using typeguard and ValidationMixin
MVrachev Mar 24, 2021
0559bbc
Add lines and file count information
MVrachev Mar 29, 2021
3f3fadb
Fix typo
MVrachev Mar 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 16 additions & 46 deletions docs/adr/0007-validation-guideliness.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,28 +35,28 @@ attributes in the middle of function execution.

## Pros, Cons, and Considerations of the Options

Here is how all of our options compare against our requirements:

| Number | Requirement | ValidationMixin | pydantic | marshmallow |
| ----------- | ----------- | ----------- | ----------- | ----------- |
| 1 | Validate function args everywhere | Limited | ✓ | Limited |
| 2 | Custom deeper validation | ✓ | ✓ | ✓ |
| 3 | Performance overhead | Minimal. | [Fastest](https://pydantic-docs.helpmanual.io/benchmarks/) | [Slower](https://pydantic-docs.helpmanual.io/benchmarks/) |
| 4 | Number of new depedencies | 0 | 2 | 1 |
MVrachev marked this conversation as resolved.
Show resolved Hide resolved
| 5 | Support for all python versions | ✓ | ✓ | ✓ |
| 6 | Code reuse for validation | ✓ | ✓ | ✓ |
| 7 | Way to invoke all validators | ✓ | ✓ | ✓ |

Bellow, in the following sections, there are additional pros, cons, and
considerations for each of the options.

### Option 1: Usage of a ValidationMixin

**Note:** All pros, cons, and considerations are documented with the assumption
we would implement the `ValidationMixin` the same way it is implemented in
[in-toto](https://github.com/in-toto) until version 1.0.1 (the latest
version at the time of writing.)

Here is how this option compares against our
[requirements](#decision-drivers-and-requirements):

| Number | Stance |
| ----------- | ----------- |
| 1 | It's limited on what it can validate. |
| 2 | Yes, it does allow that. |
| 3 | It contains only the code we need, so the overhead is minimal. |
| 4 | It doesn't add any dependencies. |
| 5 | Yes, it does support all of our python versions. |
| 6 | Yes, it does allow that. |
| 7 | Yes, it does allow that. |

Additional thoughts:

* Bad, because there could be different code paths and return statements, and as
a consequence there could be a code path that doesn't call `validate()`.

Expand Down Expand Up @@ -97,7 +97,7 @@ class User(ValidationMixin):
perspective to inherit `ValidationMixin` from classes without a "IS A"
relationship with it.
MVrachev marked this conversation as resolved.
Show resolved Hide resolved

* Consideration (*related to point 1*): if we use this option, we are limited on
* Consideration (*related to requirement 1*): if we use this option, we are limited on
what can be validated.
With the `in-toto` implementation of the `ValidationMixin`, we can only validate
class attributes inside class methods.
Expand All @@ -113,21 +113,6 @@ should be combined with custom "setter" properties.

### Option 2: Usage of a third-party library called "pydantic"

Here is how this option compares against our
[requirements](#decision-drivers-and-requirements):

| Number | Stance |
| ----------- | ----------- |
| 1 | It's not limited to what it can validate. |
| 2 | Yes, it allows that through a `@validator` decorator. |
| 3 | The fastest validation library according to [them](https://pydantic-docs.helpmanual.io/benchmarks/). |
| 4 | It adds two dependencies. |
| 5 | Yes, it does support all of our python versions. |
| 6 | Yes, it does allow that. |
| 7 | It allows it, but with not very intuitive manner. |

Additional thoughts:

* Good, because it's flexible:
1. There is a `@validate_arguments` decorator which allows us to decide which
functions to validate and the ability to validate functions outside classes.
Expand Down Expand Up @@ -208,21 +193,6 @@ class RepositoryManager(BaseModel):

### Option 3: Usage of a third-part library called "marshmallow"

Here is how this option compares against our
[requirements](#decision-drivers-and-requirements):

| Number | Stance |
| ----------- | ----------- |
| 1 | It can validate only class attributes. |
| 2 | Yes, it allows that. |
| 3 | Likely slower than `pydanitc` (according to [pydantic](https://pydantic-docs.helpmanual.io/benchmarks/)). |
| 4 | It adds 1 additional dependency. |
| 5 | Yes, it does support all of our python versions. |
| 6 | Yes, it does allow that. |
| 7 | Yes, it allows that through `validate()` function. |

Additional thoughts:

* Good, because it allows for strict type checks by marking the class attributes
(or Fields as they call them) as `strict`.

Expand Down