john-bodley commented on code in PR #24488:
URL: https://github.com/apache/superset/pull/24488#discussion_r1239092173
##########
tests/integration_tests/sqla_models_tests.py:
##########
@@ -189,11 +189,6 @@ def test_extra_cache_keys(self, flask_g):
self.assertTrue(table3.has_extra_cache_key_calls(query_obj))
assert extra_cache_keys == ["abc"]
- # Cleanup
Review Comment:
CI was failing with a SQLAlchemy error which would indicate that we're
trying to delete objects which previously weren't committed to the database.
Given how these tables are constructed,
```
table3 = SqlaTable(
table_name="test_has_no_extra_cache_keys_table",
sql=query,
database=get_example_database(),
)
```
and there's no clear add/merge call it seems that SQLAlchemy's viewpoint is
right, i.e., there's no cleanup required. If I'm write then this is a win for
SIP-92. I suspect throughout the code and tests we're doing numerous database
operations inefficiently with superfluous commits, not rolling back test state,
etc.
--
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]
