rusackas commented on code in PR #41106:
URL: https://github.com/apache/superset/pull/41106#discussion_r3564805356
##########
superset/commands/dataset/duplicate.py:
##########
@@ -75,43 +75,38 @@ def run(self) -> Model:
),
status=404,
)
- table = SqlaTable(table_name=table_name, editors=editors)
+ table = SqlaTable()
Review Comment:
Not applying this one — none of the other locals in this function
(`database_id`, `table_name`, `editors`, `database`) carry explicit annotations
either, and mypy already infers `SqlaTable` correctly from the constructor call
(confirmed green in `pre-commit run mypy`). Adding it just to this one variable
would be inconsistent with the surrounding style.
##########
superset/commands/dataset/duplicate.py:
##########
@@ -75,43 +75,38 @@ def run(self) -> Model:
),
status=404,
)
- table = SqlaTable(table_name=table_name, editors=editors)
+ table = SqlaTable()
+ table.override(self._base_model)
+ table.table_name = table_name
+ table.editors = editors
table.database = database
- table.schema = self._base_model.schema
- table.catalog = self._base_model.catalog
- table.template_params = self._base_model.template_params
- table.normalize_columns = self._base_model.normalize_columns
- table.always_filter_main_dttm =
self._base_model.always_filter_main_dttm
table.is_sqllab_view = True
- table.sql = self._base_model.sql.strip().strip(";")
+ if table.sql:
+ table.sql = table.sql.strip().strip(";")
db.session.add(table)
- cols = []
- for config_ in self._base_model.columns:
- column_name = config_.column_name
- col = TableColumn(
- column_name=column_name,
- verbose_name=config_.verbose_name,
- expression=config_.expression,
+ table.columns = [
+ TableColumn(
+ column_name=c.column_name,
+ verbose_name=c.verbose_name,
+ expression=c.expression,
filterable=True,
groupby=True,
Review Comment:
Good catch — applied. `filterable`/`groupby` now copy from the source column
instead of being hardcoded to `True` (13728e7b56), with a regression test
asserting non-default flags survive the clone.
##########
superset/commands/dataset/duplicate.py:
##########
@@ -75,43 +75,38 @@ def run(self) -> Model:
),
status=404,
)
- table = SqlaTable(table_name=table_name, editors=editors)
+ table = SqlaTable()
+ table.override(self._base_model)
+ table.table_name = table_name
+ table.editors = editors
table.database = database
- table.schema = self._base_model.schema
- table.catalog = self._base_model.catalog
- table.template_params = self._base_model.template_params
- table.normalize_columns = self._base_model.normalize_columns
- table.always_filter_main_dttm =
self._base_model.always_filter_main_dttm
table.is_sqllab_view = True
- table.sql = self._base_model.sql.strip().strip(";")
+ if table.sql:
+ table.sql = table.sql.strip().strip(";")
db.session.add(table)
- cols = []
- for config_ in self._base_model.columns:
- column_name = config_.column_name
- col = TableColumn(
- column_name=column_name,
- verbose_name=config_.verbose_name,
- expression=config_.expression,
+ table.columns = [
+ TableColumn(
+ column_name=c.column_name,
+ verbose_name=c.verbose_name,
+ expression=c.expression,
filterable=True,
groupby=True,
- is_dttm=config_.is_dttm,
- type=config_.type,
- description=config_.description,
+ is_dttm=c.is_dttm,
+ type=c.type,
+ description=c.description,
)
- cols.append(col)
- table.columns = cols
- mets = []
- for config_ in self._base_model.metrics:
- metric_name = config_.metric_name
- met = SqlMetric(
- metric_name=metric_name,
- verbose_name=config_.verbose_name,
- expression=config_.expression,
- metric_type=config_.metric_type,
- description=config_.description,
+ for c in self._base_model.columns
+ ]
+ table.metrics = [
+ SqlMetric(
+ metric_name=m.metric_name,
+ verbose_name=m.verbose_name,
+ expression=m.expression,
+ metric_type=m.metric_type,
+ description=m.description,
)
Review Comment:
Good catch — applied. `d3format`, `currency`, `warning_text`, and `extra`
now carry over to the cloned metric (13728e7b56), with a regression test
covering it.
--
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]