mistercrunch commented on code in PR #33547:
URL: https://github.com/apache/superset/pull/33547#discussion_r2101484157
##########
superset-frontend/src/components/Datasource/DatasourceModal.tsx:
##########
@@ -140,7 +140,7 @@ const DatasourceModal:
FunctionComponent<DatasourceModalProps> = ({
cache_timeout:
datasource.cache_timeout === '' ? null : datasource.cache_timeout,
is_sqllab_view: datasource.is_sqllab_view,
- template_params: datasource.template_params,
+ template_params: datasource.template_params === '' ? null :
datasource.template_params,
Review Comment:
ideally we align the types and avoid the lazy `Record<string, any>` set
above, seems the right type is here ->
https://github.com/apache/superset/blob/master/superset-frontend/src/features/datasets/types.ts#L72
, let's use it at line 123
Now should template_params be a string? Unclear, maybe, maybe not, ideally
the components where that came from always returns a proper JSON serializable,
but that's probably more work.
Now if the backend is in charge of serializing the string, if feels like the
fix should be done there ideally. Not again the quickfix here but should
include a `// TODO change type to {...} and push validation in the component`.
Trying to figure out where in the backend the serialization is done ...do
you have a stacktrace of the backend failing?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
