Skip to content

Validate component properties #264 - March 1 #452

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

Closed
wants to merge 46 commits into from
Closed
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
5ac225b
Add `numpy`, `pandas`, `Cerberus` to requirements.
rmarren1 Nov 8, 2018
b1ef131
Add cerberus to setup.py install_requires
rmarren1 Nov 8, 2018
9e5c370
Add a `suppress_validation_exceptions` option to `dash.Dash`.
rmarren1 Nov 8, 2018
26e425a
Add validation exception definitions.
rmarren1 Nov 8, 2018
10498d2
Add the `DashValidator` class.
rmarren1 Nov 8, 2018
d943e2e
Add a modular decode hook for loading `metadata.json` files.
rmarren1 Nov 8, 2018
b12c73d
Add a `validate` method to the base component class.
rmarren1 Nov 8, 2018
c123915
Add schema generation logic.
rmarren1 Nov 8, 2018
6381b54
Integrate schema generation with the Python class string generation.
rmarren1 Nov 8, 2018
7ac65f2
Re-name validation functions to be more explicit.
rmarren1 Nov 8, 2018
1ba24f7
Validate callback outputs.
rmarren1 Nov 8, 2018
f21372b
Validate app initial layout.
rmarren1 Nov 8, 2018
398950d
Update test component code (react 16, children PropType, more props)
rmarren1 Nov 8, 2018
83f70d7
Re-build metadata_test.py
rmarren1 Nov 8, 2018
0883290
Update base component tests that would have broken given component ch…
rmarren1 Nov 8, 2018
422c2e2
Add test for the generated schema.
rmarren1 Nov 8, 2018
3ff6dd4
Update component loader tests.
rmarren1 Nov 8, 2018
b81084a
Add component validation tests.
rmarren1 Nov 8, 2018
27d7da5
Run component validation tests in CI.
rmarren1 Nov 8, 2018
7aa7060
Add some lines forgotten during rebase.
rmarren1 Nov 8, 2018
28b8a13
Pylint fixes.
rmarren1 Nov 8, 2018
ee12470
Flake8 fixes.
rmarren1 Nov 8, 2018
e817d75
Make style changes in response to code review.
rmarren1 Nov 8, 2018
6b8cfd3
Lint fixes.
rmarren1 Nov 8, 2018
2c2faf5
Fixes for review.
rmarren1 Nov 12, 2018
5d560e6
Do not suppress callback exceptions when in `debug=False`.
rmarren1 Nov 12, 2018
1f48149
Pylint, Flake8 fixes.
rmarren1 Nov 12, 2018
29c363c
Add back circleci flake8 config.
rmarren1 Nov 17, 2018
783d2c2
Flake8 fixes in tests
rmarren1 Nov 17, 2018
8e96ed2
Lock dev-requirements `dash` version to validation RC.
rmarren1 Nov 24, 2018
79b5251
Test that sets do not validate as lists.
rmarren1 Nov 25, 2018
1bf1036
Change list validation to handle sets properly.
rmarren1 Nov 25, 2018
d842651
Test that arbitraty types can be validated as a number without error.
rmarren1 Nov 25, 2018
5936dfa
Only print validation suppression message once.
rmarren1 Nov 30, 2018
dcc3c40
Rebase on dash 31.
rmarren1 Nov 30, 2018
06d03c7
Delete duplicate files from rebase.
rmarren1 Nov 30, 2018
83f501a
Properly remove schema for python version cross compatibility.
rmarren1 Nov 30, 2018
1dae812
Fix prop filtering in validate method
rmarren1 Nov 30, 2018
7d00b80
:hocho: old method of validating required props
rmarren1 Nov 30, 2018
86d5e7c
Rebase.
rmarren1 Dec 14, 2018
f7312bf
Merge branch 'master' into validate2
rmarren1 Dec 18, 2018
5a50ee8
Merge with current master
rmarren1 Feb 21, 2019
792ef39
Add test_component_validation test to test.sh
rmarren1 Feb 21, 2019
598a432
Add generated schema to new base class, fix tests.
rmarren1 Feb 21, 2019
aa49488
Validation tests require numpy and pandas.
rmarren1 Feb 21, 2019
56c2873
Remove duplicate key
rmarren1 Feb 21, 2019
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
3 changes: 1 addition & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ jobs:
command: |
. venv/bin/activate
pylint dash setup.py --rcfile=$PYLINTRC
pylint tests -d all -e C0410,C0411,C0412,C0413,W0109
flake8 dash setup.py
flake8 --ignore=E123,E126,E501,E722,E731,F401,F841,W503,W504 --exclude=metadata_test.py tests
Copy link
Contributor

Choose a reason for hiding this comment

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


- run:
name: Run tests
Expand All @@ -51,6 +49,7 @@ jobs:
python --version
python -m unittest tests.development.test_base_component
python -m unittest tests.development.test_component_loader
python -m unittest tests.development.test_component_validation
python -m unittest tests.test_integration
python -m unittest tests.test_resources
python -m unittest tests.test_configs
Expand Down
3 changes: 3 additions & 0 deletions .circleci/requirements/dev-requirements-py37.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ mock
tox
tox-pyenv
six
numpy
pandas
plotly>=2.0.8
requests[security]
flake8
pylint==2.1.1
astroid==2.0.4
Cerberus
3 changes: 3 additions & 0 deletions .circleci/requirements/dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ tox
tox-pyenv
mock
six
numpy
pandas
plotly>=2.0.8
requests[security]
flake8
pylint==1.9.2
Cerberus
116 changes: 107 additions & 9 deletions dash/dash.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
import pkgutil
import warnings
import re
import pprint

from functools import wraps
from textwrap import dedent

import plotly
import dash_renderer
Expand All @@ -20,6 +22,8 @@
from .dependencies import Event, Input, Output, State
from .resources import Scripts, Css
from .development.base_component import Component
from .development.validator import (DashValidator,
generate_validation_error_message)
from . import exceptions
from ._utils import AttributeDict as _AttributeDict
from ._utils import interpolate_str as _interpolate
Expand Down Expand Up @@ -84,6 +88,7 @@ def __init__(
external_scripts=None,
external_stylesheets=None,
suppress_callback_exceptions=None,
suppress_validation_exceptions=None,
components_cache_max_age=None,
**kwargs):

Expand Down Expand Up @@ -126,6 +131,10 @@ def __init__(
'suppress_callback_exceptions',
suppress_callback_exceptions, env_configs, False
),
'suppress_validation_exceptions': _configs.get_config(
'suppress_validation_exceptions',
suppress_validation_exceptions, env_configs, False
),
'routes_pathname_prefix': routes_pathname_prefix,
'requests_pathname_prefix': requests_pathname_prefix,
'include_assets_files': _configs.get_config(
Expand Down Expand Up @@ -572,7 +581,7 @@ def react(self, *args, **kwargs):
'Use `callback` instead. `callback` has a new syntax too, '
'so make sure to call `help(app.callback)` to learn more.')

def _validate_callback(self, output, inputs, state, events):
def _validate_callback_definition(self, output, inputs, state, events):
# pylint: disable=too-many-branches
layout = self._cached_layout or self._layout_value()

Expand Down Expand Up @@ -710,7 +719,7 @@ def _validate_callback(self, output, inputs, state, events):
output.component_id,
output.component_property).replace(' ', ''))

def _validate_callback_output(self, output_value, output):
def _debug_callback_serialization_error(self, output_value, output):
valid = [str, dict, int, float, type(None), Component]

def _raise_invalid(bad_val, outer_val, bad_type, path, index=None,
Expand Down Expand Up @@ -828,7 +837,7 @@ def _validate_value(val, index=None):
# relationships
# pylint: disable=dangerous-default-value
def callback(self, output, inputs=[], state=[], events=[]):
self._validate_callback(output, inputs, state, events)
self._validate_callback_definition(output, inputs, state, events)

callback_id = '{}.{}'.format(
output.component_id, output.component_property
Expand All @@ -850,13 +859,11 @@ def callback(self, output, inputs=[], state=[], events=[]):

def wrap_func(func):
@wraps(func)
def add_context(*args, **kwargs):

output_value = func(*args, **kwargs)
def add_context(validated_output):
response = {
'response': {
'props': {
output.component_property: output_value
output.component_property: validated_output
}
}
}
Expand All @@ -867,7 +874,10 @@ def add_context(*args, **kwargs):
cls=plotly.utils.PlotlyJSONEncoder
)
except TypeError:
self._validate_callback_output(output_value, output)
self._debug_callback_serialization_error(
validated_output,
output
)
raise exceptions.InvalidCallbackReturnValue('''
The callback for property `{property:s}`
of component `{id:s}` returned a value
Expand All @@ -884,6 +894,7 @@ def add_context(*args, **kwargs):
mimetype='application/json'
)

self.callback_map[callback_id]['func'] = func
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do that in two difference part ? Couldn't your additions in dispatch be in add_context ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We would have to pass namespace, component_id, component_property, to this since that info is available in dispatch. I originally thought this might clash with the other arguments (e.g. if you had a property called namespace) although I now think this would be fine, but it might make it difficult if we want to support passing props with keyword arguments in the future.

self.callback_map[callback_id]['callback'] = add_context

return add_context
Expand Down Expand Up @@ -912,7 +923,85 @@ def dispatch(self):
c['id'] == component_registration['id']
][0])

return self.callback_map[target_id]['callback'](*args)
output_value = self.callback_map[target_id]['func'](*args)
Copy link
Contributor

Choose a reason for hiding this comment

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

The old call to the functions also had **kwargs, is that handled somewhere else or just not needed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it did, this was the last change to that line and it had *args


# Only validate if we get required information from renderer
# and validation is not turned off by user
if (
not self.config.suppress_validation_exceptions and
'namespace' in output and
'type' in output
):
Copy link
Contributor

Choose a reason for hiding this comment

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

This would fit without the ()

if not self.config.suppress_validation_exceptions and \
                'namespace' in output and 'type' in output:

# Python2.7 might make these keys and values unicode
namespace = str(output['namespace'])
component_type = str(output['type'])
component_id = str(output['id'])
component_property = str(output['property'])
callback_func_name = self.callback_map[target_id]['func'].__name__
self._validate_callback_output(namespace, component_type,
component_id, component_property,
callback_func_name,
args, output_value)

return self.callback_map[target_id]['callback'](output_value)

def _validate_callback_output(self, namespace, component_type,
component_id, component_property,
callback_func_name, args, value):
module = sys.modules[namespace]
component = getattr(module, component_type)
# pylint: disable=protected-access
validator = DashValidator({
component_property: component._schema.get(component_property, {})
})
valid = validator.validate({component_property: value})
if not valid:
error_message = dedent("""\

A Dash Callback produced an invalid value!

Dash tried to update the `{component_property}` prop of the
`{component_name}` with id `{component_id}` by calling the
`{callback_func_name}` function with `{args}` as arguments.

This function call returned `{value}`, which did not pass
validation tests for the `{component_name}` component.

The expected schema for the `{component_property}` prop of the
`{component_name}` component is:

***************************************************************
{component_schema}
***************************************************************

The errors in validation are as follows:

""").format(
component_property=component_property,
component_name=component.__name__,
component_id=component_id,
callback_func_name=callback_func_name,
args='({})'.format(", ".join(map(repr, args))),
Copy link
Contributor

Choose a reason for hiding this comment

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

😸

value=value,
component_schema=pprint.pformat(
component._schema[component_property]
)
)

raise exceptions.CallbackOutputValidationError(
generate_validation_error_message(
validator.errors,
0,
error_message
)
)
# Must also validate initialization of newly created components
if component_property == 'children':
if isinstance(value, Component):
value.validate()
for component in value.traverse():
if isinstance(component, Component):
component.validate()

def _validate_layout(self):
if self.layout is None:
Expand All @@ -929,6 +1018,11 @@ def _validate_layout(self):

component_ids = {layout_id} if layout_id else set()
for component in to_validate.traverse():
if (
not self.config.suppress_validation_exceptions and
isinstance(component, Component)
):
component.validate()
component_id = getattr(component, 'id', None)
if component_id and component_id in component_ids:
raise exceptions.DuplicateIdError(
Expand Down Expand Up @@ -1054,5 +1148,9 @@ def run_server(self,
:return:
"""
debug = self.enable_dev_tools(debug, dev_tools_serve_dev_bundles)
if not debug:
# Do not throw debugging exceptions in production.
self.config.suppress_validation_exceptions = True
self.config.suppress_callback_exceptions = True
Copy link
Contributor

Choose a reason for hiding this comment

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

⚡ Only suppress the props validation, we still want to validate the callbacks and it's not an expensive check performance wise.

self.server.run(port=port, debug=debug,
**flask_run_options)
Loading