testcontainers/testcontainers-python

Design question / is a hard dependency on SqlAlchemy really necessary?

logicbomb opened this issue · 8 comments

Hi,

It seems that the _connect method is causing the DbContainer project to take a hard dependency on SqlAlchemy. This is troublesome for those of us not using that package. Is it meant to do more than verify the database container is up & running? If not, maybe there's a more neutral way of checking like using pg_isready?

thanks,
Jason

Yes, that's a good point. We should move away from a hard dependency on sqlalchemy as you suggested. Do you have thoughts on how to integrate that change? A PR would be very welcome!

i thought to use popen to execute the pg_isready command, does that sound like a reasonable approach?

i thought to use popen to execute the pg_isready command, does that sound like a reasonable approach?

Based on my experience maintaining TC for .NET, waiting for pg_isready to succeed is not always reliable. There were instances where the .NET Npgsql client could not establish a connection. Since I wait for specific log messages, I have not encountered any connection issues.

I put together a WIP here: https://github.com/logicbomb/testcontainers-python/pull/1/files

You can see 2 possible implementations, one based on pg_isready the other checking logs as described by @HofmeisterAn. Using pg_isready is much less code and seems to work, but I'm happy to go with either.

Also, would you all require a change to the way all databases are verified, or would you accept a change only for Postgres?

You may want to have a look at the wait_for_logs function here. That should reduce the amount of code required.

ooh, wait_for_logs is nice. i take it you prefer that approach?

Yes, using wait_for_logs could be a good option.