[PATCH] D37360: Keep sqlalchemy session separate from database objects

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls added a comment.

I have felt before that LNT's connection and interaction with the database is too obscure in the code.
Therefore, I like this proposal, even if it means passing around session objects more.

I've even wondered before if we shouldn't just get rid of SqlAlchemy and write sql directly to allow us to speed up some of the heavy queries.
That would make the interaction with the database in LNT's code clearer again, removing an obfuscating layer of indirection.
However, I do see that with now supporting 3 database engines (sqlite, postgres, mysql), we may not want to have to reinvent the wheel on handling the database differences that SqlAlchemy already handles.
Anyway, the SqlAlchemy thought is clearly independent of this particular change.

I've been out of the loop on python & database interactions for too many years to be able to properly review this, but as said above, I do like the direction this is taking LNT's design/code base.


Repository:
  rL LLVM

https://reviews.llvm.org/D37360