bkyryliuk commented on a change in pull request #13739:
URL: https://github.com/apache/superset/pull/13739#discussion_r605769319
##########
File path: superset/connectors/sqla/models.py
##########
@@ -873,7 +858,7 @@ def adhoc_metric_to_sqla(
if table_column:
sqla_column = table_column.get_sqla_col()
else:
- sqla_column = column(column_name)
+ sqla_column = table(self.table_name,
column(column_name)).c[column_name]
Review comment:
it would be nice to address codecov comment:
`Added line #L861 was not covered by tests`
##########
File path: tests/conftest.py
##########
@@ -73,13 +73,22 @@ def drop_from_schema(engine: Engine, schema_name: str):
def setup_presto_if_needed():
backend = app.config["SQLALCHEMY_EXAMPLES_URI"].split("://")[0]
+ database = get_example_database()
+ extra = database.get_extra()
+
if backend == "presto":
# decrease poll interval for tests
- presto_poll_interval = app.config["PRESTO_POLL_INTERVAL"]
- extra = f'{{"engine_params": {{"connect_args": {{"poll_interval":
{presto_poll_interval}}}}}}}'
- database = get_example_database()
- database.extra = extra
- db.session.commit()
+ extra = {
+ **extra,
+ "engine_params": {
+ "connect_args": {"poll_interval":
app.config["PRESTO_POLL_INTERVAL"]}
+ },
+ }
+ else:
+ # remove `poll_interval` from databases that do not support it
+ extra = {**extra, "engine_params": {}}
Review comment:
`extra = {**extra, "engine_params": {}}` - I think it is better to
figure out where poll_interval is set and not to set it for the databases that
do not support it.
`setup_presto_if_needed` function should not modify other database settings
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
