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 for PostgreSQL (though not MySQL or SQLite—which is a little
perplexing) with 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,
```python
table3 = SqlaTable(
table_name="test_has_no_extra_cache_keys_table",
sql=query,
database=get_example_database(),
)
```
and there's no clear add/commit call it seems that SQLAlchemy's viewpoint is
right, i.e., there's no cleanup required. If I'm right 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]
