ferruzzi commented on issue #71577:
URL: https://github.com/apache/airflow/issues/71577#issuecomment-5298423453
Agreed on the separate PR for the enforcer for sure. Plus, if we do the
detector first, we can use that to generate the starting allowlist for the
enforcer, which is nice.
I think narrowing the detector may be a mistake though, these are the ones
we know about, doesn't mean they are the only ones. Maybe build it to monitor
everything and trim it down if timing is a concern? As you say, 1ms is
nothing. `devel-common/src/tests_common/test_utils/db.py` has 26 `clear_db_*`
helpers hitting almost every table in the database, maybe we can generate a
list of what those helpers hit to use as the scope somehow so we don't have to
manually update the table list?
Did you benchmark on six individual queries, or as a single batch? I
suspect the bulk of that 1ms (even as small as that is) was taken by the data
roundtrip and not the actual query, but since we only need row counts, if you
batch it all into one query that might be more efficient. I'm not great with
DB stuff, but something along the lines of this (untested pseudo-code) might
actually just check the entire db just as fast without bothering with a list of
tables if we're already planning to hit almost all of them anyway:
```
counts = []
for table in Base.metadata.sorted_tables:
counts.append(select(literal(table.name),
func.count()).select_from(table))
snapshot = session.execute(union_all(*counts)).all()
```
And one last thought I have not put any research into: are there any cases
where we WANT per-test leaks? Maybe have a look and see if we ave any
per-class fixtures that persist across a few tests but then do actually clean
up after themselves at the end of the suite.
Another point about this whole approach is that basing it off row counts
isn't foolproof. If I add one and remove another, this will clear despite my
monkeying... so maybe there's a better way, or maybe row count is "good
enough", or maybe we can get this going and expand it with a per-row hash later
or something.... As I said, I haven't really thought this through all the way,
there are possibly pitfalls still out there to consider...
--
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]
