-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
plugin.api.validate: refactor, turn into package #4514
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
Conversation
38de126 to
ae8757e
Compare
|
Example of the ValidationError output with this diff applied to the YouTube plugin, which breaks its validation schema: diff --git a/src/streamlink/plugins/youtube.py b/src/streamlink/plugins/youtube.py
index 13a5872e..bc086bf2 100644
--- a/src/streamlink/plugins/youtube.py
+++ b/src/streamlink/plugins/youtube.py
@@ -157,7 +157,7 @@ class YouTube(Plugin):
"microformat": validate.all(
validate.any(
validate.all(
- {"playerMicroformatRenderer": dict},
+ {"playerMicroformatRenderer": float},
validate.get("playerMicroformatRenderer")
),
validate.all(Compare that to the error message of the same diff applied to |
898414c to
40c9fb1
Compare
|
I've renamed the sub-modules in my latest push, so that there's no naming conflict between the This should be ready for review now. The other incomplete goals I've listed in the OP are not important. Please see the invidual commits and their commit messages for the motivation of these changes. As you can see, I applied some additional changes in the first commit while splitting up the module, but I could change that and add more simple commits, if that helps reviewing. Also, a still unresolved question is whether |
|
I'm going to include a rewrite of the tests in this PR (not yet finished). There are a couple of things I noticed while rewriting the tests which should be improved. There's a broken type check in the TransformSchema validation, some validate calls should be wrapped for better error messages, and I think some stuff should be re-arranged. |
Turn into SchemaContainer subclasses
Turn into validation schema with a singledispatch function. This is required for the upcoming module split.
Remove callable check from the base validate singledispatch function and register the abc.Callable type instead. Also fix type validation in the transform schema.
Add missing tail attribute and clone child nodes
Inherit from the all-schema directly to avoid having an unnecessary singledispatch type definition.
Turn module into package with multiple logical sub-modules:
- Define a public interface in the package's `__init__` module
- Split validation schemas, validators and validate logic
- schemas: classes which register attributes used by their
respective `validate` implementations
- validators: functions which can internally call `validate`
and which return something that can be validated
- validate: singledispatch functions which implement the validation
logic for schemas and various other types
- Rename validation schemas for better internal references
- Rename singledispatch methods
Other clean-up work:
- Update comments and fix grammar
- Add type annotations
- Use f-strings
- Use `str` instead of the `text` alias
- Simplify some code blocks
- Rearrange classes and functions
- Rephrase certain error messages
- Add a few more tests for better code coverage
- Implement `ValidationError` - Inherit from `ValueError` to preserve backwards compatiblity - Allow collecting multiple errors (AnySchema) - Keep an error stack of parent `ValidationError`s or other exceptions - Format error stack when converting error to string - Raise `ValidationError` instead of `ValueError` - Add error contexts where it makes sense - Add schema names to error instances - Add and update tests
- Change signature of `ValidationError` to be able to pass template strings and template variables which can get truncated individually. - Make error messages consistent by using string representations for template variables where it makes sense
Completely rewrite tests using pytest, with full coverage
40c9fb1 to
3153e8d
Compare
|
Apologies for the back-and-forth with this, but as said, there were a couple of things which I felt needed to be adjusted. I've rebased the PR branch and split up everything into more logical commits for easier review. The latest push now also includes a complete rewrite of the tests based on pytest with a proper separation of the tested schemas and validations, and it now covers 100%. Every commit of this branch should however be valid on its own. I am pretty much satisfied with the changes now and unless there's a bug, I won't be changing anything here anymore. There's still some stuff that can be improved in the future though. |
Add missing tail attribute and clone child nodes
Turn into SchemaContainer subclasses
Turn into validation schema with a singledispatch function. This is required for the upcoming module split.
Remove callable check from the base validate singledispatch function and register the abc.Callable type instead. Also fix type validation in the transform schema.
Inherit from the all-schema directly to avoid having an unnecessary singledispatch type definition.
Remove callable check from the base validate singledispatch function and register the abc.Callable type instead. Also fix type validation in the transform schema.
Inherit from the all-schema directly to avoid having an unnecessary singledispatch type definition.
Turn module into package with multiple logical sub-modules:
- Define a public interface in the package's `__init__` module
- Split validation schemas, validators and validate logic
- schemas: classes which register attributes used by their
respective `validate` implementations
- validators: functions which can internally call `validate`
and which return something that can be validated
- validate: singledispatch functions which implement the validation
logic for schemas and various other types
- Rename validation schemas for better internal references
- Rename singledispatch methods
Other clean-up work:
- Update comments and fix grammar
- Add type annotations
- Use f-strings
- Use `str` instead of the `text` alias
- Simplify some code blocks
- Rearrange classes and functions
- Rephrase certain error messages
- Add a few more tests for better code coverage
- Implement `ValidationError`
- Inherit from `ValueError` to preserve backwards compatiblity
- Allow collecting multiple errors (AnySchema)
- Keep an error stack of parent `ValidationError`s or other exceptions
- Format error stack when converting error to string
- Raise `ValidationError` instead of `ValueError`
- Add error contexts where it makes sense
- Add schema names to error instances
- Add and update tests
- Change signature of `ValidationError` to be able to pass template strings and template variables which can get truncated individually. - Make error messages consistent by using string representations for template variables where it makes sense
Completely rewrite tests using pytest, with full coverage
Turn module into package with multiple logical sub-modules:
- Define a public interface in the package's `__init__` module
- Split validation schemas, validators and validate logic
- schemas: classes which register attributes used by their
respective `validate` implementations
- validators: functions which can internally call `validate`
and which return something that can be validated
- validate: singledispatch functions which implement the validation
logic for schemas and various other types
- Rename validation schemas for better internal references
- Rename singledispatch methods
Other clean-up work:
- Update comments and fix grammar
- Add type annotations
- Use f-strings
- Use `str` instead of the `text` alias
- Simplify some code blocks
- Rearrange classes and functions
- Rephrase certain error messages
- Add a few more tests for better code coverage
- Implement `ValidationError`
- Inherit from `ValueError` to preserve backwards compatiblity
- Allow collecting multiple errors (AnySchema)
- Keep an error stack of parent `ValidationError`s or other exceptions
- Format error stack when converting error to string
- Raise `ValidationError` instead of `ValueError`
- Add error contexts where it makes sense
- Add schema names to error instances
- Add and update tests
- Change signature of `ValidationError` to be able to pass template strings and template variables which can get truncated individually. - Make error messages consistent by using string representations for template variables where it makes sense
Completely rewrite tests using pytest, with full coverage
Turn module into package with multiple logical sub-modules:
- Define a public interface in the package's `__init__` module
- Split validation schemas, validators and validate logic
- schemas: classes which register attributes used by their
respective `validate` implementations
- validators: functions which can internally call `validate`
and which return something that can be validated
- validate: singledispatch functions which implement the validation
logic for schemas and various other types
- Rename validation schemas for better internal references
- Rename singledispatch methods
Other clean-up work:
- Update comments and fix grammar
- Add type annotations
- Use f-strings
- Use `str` instead of the `text` alias
- Simplify some code blocks
- Rearrange classes and functions
- Rephrase certain error messages
- Add a few more tests for better code coverage
- Implement `ValidationError`
- Inherit from `ValueError` to preserve backwards compatiblity
- Allow collecting multiple errors (AnySchema)
- Keep an error stack of parent `ValidationError`s or other exceptions
- Format error stack when converting error to string
- Raise `ValidationError` instead of `ValueError`
- Add error contexts where it makes sense
- Add schema names to error instances
- Add and update tests
- Change signature of `ValidationError` to be able to pass template strings and template variables which can get truncated individually. - Make error messages consistent by using string representations for template variables where it makes sense
Completely rewrite tests using pytest, with full coverage
A lot of changes with lots of details. Please see the individual commit messages for the explanation and motivation of these changes. The added tests for the newly added
ValidationErrorclass also make the motivation a bit more clear.Backwards compatiblity is / should be kept.
plugin.api.validatemoduleschemas: classes which register attributes used by their respectivevalidateimplementationsvalidators: functions which can internally callvalidateand which return something that can be validatedvalidate: singledispatch functions which implement the validation logic for schemas and various other typesSchema,AllSchema,AnySchema,GetItemSchemaandCallableXmlElementSchemasingledispatchvalidation functionsValidationError(inherit fromValueErrorfor backwards compatibility)AnySchema) and keep an error stack (error contexts)ValidationErrorinstances, for better legibility on error outputAdd a few more convenience schemas/validators for patterns that come up all the time(will check later)Add basic documentation(will check later)