Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
5ac225bb1ef1319e5c37026e425a10498d2d943e2eb12c73dc1239156381b547ac65f21ba24f7f21372b398950d83f70d70883290422c2e23ff6dd4b81084a27d7da57aa706028b8a13ee12470e817d756b8cfd32c2faf55d560e61f4814929c363c783d2c28e96ed279b52511bf1036d8426515936dfadcc3c4006d03c783f501a1dae8127d00b8086d5e7cf7312bf5a50ee8792ef39598a432aa4948856c2873File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -14,3 +14,6 @@ requests[security] | ||
| flake8 | ||
| pylint==2.2.2 | ||
| astroid==2.1.0 | ||
| Cerberus | ||
| numpy | ||
| pandas | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -14,3 +14,6 @@ plotly==3.6.1 | ||
| requests[security] | ||
| flake8 | ||
| pylint==1.9.4 | ||
| Cerberus | ||
| numpy | ||
| pandas | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -11,6 +11,7 @@ | ||
| import threading | ||
| import warnings | ||
| import re | ||
| import pprint | ||
| import logging | ||
| from functools import wraps | ||
| @@ -25,6 +26,8 @@ | ||
| from .dependencies import Input, Output, State | ||
| from .resources import Scripts, Css | ||
| from .development.base_component import Component, ComponentRegistry | ||
| 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 | ||
| @@ -69,6 +72,27 @@ | ||
| _re_index_config_id = re.compile(r'id="_dash-config"') | ||
| _re_index_scripts_id = re.compile(r'src=".*dash[-_]renderer.*"') | ||
| _callback_validation_error_template = """ | ||
| 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: | ||
| """ | ||
| # pylint: disable=too-many-instance-attributes | ||
| # pylint: disable=too-many-arguments, too-many-locals | ||
| @@ -92,6 +116,7 @@ def __init__( | ||
| external_scripts=None, | ||
| external_stylesheets=None, | ||
| suppress_callback_exceptions=None, | ||
| suppress_validation_exceptions=None, | ||
| components_cache_max_age=None, | ||
| **kwargs): | ||
| @@ -128,6 +153,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( | ||
| @@ -765,7 +794,7 @@ def _validate_callback(self, output, inputs, state): | ||
| 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, | ||
| @@ -901,13 +930,11 @@ def callback(self, output, inputs=[], state=[]): | ||
| 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 | ||
| } | ||
| } | ||
| } | ||
| @@ -918,7 +945,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 | ||
| @@ -935,6 +965,7 @@ def add_context(*args, **kwargs): | ||
| mimetype='application/json' | ||
| ) | ||
| self.callback_map[callback_id]['func'] = func | ||
| self.callback_map[callback_id]['callback'] = add_context | ||
| return add_context | ||
| @@ -978,7 +1009,62 @@ 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) | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old call to the functions also had ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
| # 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: | ||
| # 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 = _callback_validation_error_template.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))), | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||
| @@ -995,6 +1081,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( | ||
| @@ -1292,6 +1383,10 @@ def run_server(self, | ||
| dev_tools_silence_routes_logging, | ||
| ) | ||
| if not debug: | ||
| # Do not throw validation exceptions in production. | ||
| self.config.suppress_validation_exceptions = True | ||
| if self._dev_tools.silence_routes_logging: | ||
| # Since it's silenced, the address don't show anymore. | ||
| host = flask_run_options.get('host', '127.0.0.1') | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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 indispatch. I originally thought this might clash with the other arguments (e.g. if you had a property callednamespace) 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.