You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/06/18 01:12:14 UTC
[GitHub] [airflow] mik-laj opened a new pull request #9365: Use strict API schemas
mik-laj opened a new pull request #9365:
URL: https://github.com/apache/airflow/pull/9365
The rules are already verified by connexion, but it's still worth forcing some unexpected errors to not be accidentally ignored.
```
:param bool strict: If `True`, raise errors if invalid data are passed in
instead of failing silently and storing the errors.
```
CC: @ephraimbuddy
---
Make sure to mark the boxes below before creating PR: [x]
- [X] Description above provides context of the change
- [X] Unit tests coverage for changes (not needed for documentation changes)
- [X] Target Github ISSUE in description if exists
- [X] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [X] Relevant documentation is updated including usage instructions.
- [X] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #9365: Use strict API schemas
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #9365:
URL: https://github.com/apache/airflow/pull/9365#issuecomment-645969223
How about using something like since we would always want the `strict=True`:
```python
class Schema(marshmallow.Schema):
def __init__(self, strict=True, **kwargs):
super(Schema, self).__init__(strict=strict, **kwargs)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #9365: Use strict API schemas
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9365:
URL: https://github.com/apache/airflow/pull/9365#issuecomment-645722005
@ephraimbuddy Good point. I will do.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #9365: Use strict API schemas
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #9365:
URL: https://github.com/apache/airflow/pull/9365#issuecomment-645721692
I also suggest we enable strict validation here: airflow/www/extensions/init_views.py. It's currently disabled
`specification='v1.yaml', base_path='/api/v1', validate_responses=True, strict_validation=False`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #9365: Use strict API schemas
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9365:
URL: https://github.com/apache/airflow/pull/9365#issuecomment-645967108
@kaxil No. This has only changed in 3.0. We use marshmallow 2.*.
https://marshmallow.readthedocs.io/en/stable/upgrading.html#schemas-are-always-strict
However, we will want to migrate to the new version soon.
https://github.com/dpgaspar/Flask-AppBuilder/pull/1382
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #9365: Use strict API schemas
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #9365:
URL: https://github.com/apache/airflow/pull/9365#issuecomment-645979068
> @kaxil This will complicate reviews because we will have to check imports. I do not see the problem that the code is repeated, because it is only one option.
>
> > Explicit is better than implicit.
>
> https://www.python.org/dev/peps/pep-0020/
We will have to remove all the class that inherits then :D We can call it `StrictSchema` etc to be explicit.
Then main reason I recommended that was to enforce that we always use `strict` as it is easy to forget that in new PR and then we end up with some of them strict and some of them not. It is easy then to add pre-commit hook to check that only `StrictSchema` is used.
Anyways @ephraimbuddy makes a good point that we use SQLALchemySchema at some places. So let's keep it as it is. ✅
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #9365: Use strict API schemas
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #9365:
URL: https://github.com/apache/airflow/pull/9365#issuecomment-645972468
@kaxil This will complicate reviews because we will have to check imports. I do not see the problem that the code is repeated, because it is only one option.
> Explicit is better than implicit.
https://www.python.org/dev/peps/pep-0020/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #9365: Use strict API schemas
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #9365:
URL: https://github.com/apache/airflow/pull/9365#issuecomment-645713975
Nice!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #9365: Use strict API schemas
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on pull request #9365:
URL: https://github.com/apache/airflow/pull/9365#issuecomment-645972701
Some Schemas uses SQLALchemySchema from marshmallow_sqlalchemy
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj merged pull request #9365: Use strict API schemas
Posted by GitBox <gi...@apache.org>.
mik-laj merged pull request #9365:
URL: https://github.com/apache/airflow/pull/9365
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil edited a comment on pull request #9365: Use strict API schemas
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on pull request #9365:
URL: https://github.com/apache/airflow/pull/9365#issuecomment-645969223
How about using something like since we would always want the `strict=True`:
```python
class Schema(marshmallow.Schema):
def __init__(self, strict=True, **kwargs):
super().__init__(strict=strict, **kwargs)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org