villebro commented on a change in pull request #8138: [typing] add typing for
superset/connectors and superset/common
URL:
https://github.com/apache/incubator-superset/pull/8138#discussion_r318888377
##########
File path: superset/common/query_context.py
##########
@@ -40,8 +42,13 @@ class QueryContext:
to retrieve the data payload for a given viz.
"""
- cache_type = "df"
- enforce_numerical_metrics = True
+ cache_type: str = "df"
+ enforce_numerical_metrics: bool = True
+
+ datasource: BaseDatasource
+ queries: List[QueryObject]
+ force: bool
+ custom_cache_timeout: Optional[int]
Review comment:
Adding these as class attributes can be a handy fallback if the instance
attribute doesn't exist, but introduces potential for trouble, as mutating them
will flow through to other instances as well. Example
```python
a = QueryContext()
b = QueryContext()
a.queries.append(QueryObject())
b.queries # <- this now has a QueryObject instance!
```
These can be nasty to debug. Not sure if this is a problem here, but I would
be cautious of adding it unless certain that it is ok, and to me it seems like
they were left out on purpose.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]