villebro commented on code in PR #23997:
URL: https://github.com/apache/superset/pull/23997#discussion_r1189417203
##########
docs/static/resources/openapi.json:
##########
@@ -1358,11 +1346,6 @@
"nullable": true,
"type": "array"
},
- "granularity": {
- "description": "Name of temporal column used for time filtering.
For legacy Druid datasources this defines the time grain.",
- "nullable": true,
- "type": "string"
- },
Review Comment:
I still have trouble wrapping my head around these different granularities
and when they're in the main payload vs extras, but shouldn't this still be
here, only without the reference to legacy Druid? I looked at the `QueryObject`
class, and `granularity` still seems to be featured in the constructor/property
list.
Alternatively, do we need to update line 1367 which states that
`granularity_sqla` "is deprecated, use `granularity` instead."?
##########
superset/charts/post_processing.py:
##########
@@ -34,7 +34,6 @@
from superset.common.chart_data import ChartDataResultFormat
from superset.utils.core import (
- DTTM_ALIAS,
Review Comment:
I'm always thrilled to see the removal of a `DTTM_ALIAS` reference 🙂
##########
superset/viz.py:
##########
@@ -372,9 +372,7 @@ def query_obj(self) -> QueryObjectDict: # pylint:
disable=too-many-locals
del groupby[groupby_labels.index(DTTM_ALIAS)]
is_timeseries = True
- granularity = self.form_data.get("granularity") or self.form_data.get(
Review Comment:
Agreed, good reminder to NEVER have dual meanings for variables.
--
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]
