Remove sqlalchemy dependency from postgres connection check by logicbomb · Pull Request #340 · testcontainers/testcontainers-python · GitHub

I decided to include both the log check & the pg_isready check, as it seems they both need to be successful in order to enure the database is running.

I don't think the driver parameter belongs in Postgres.__init__ method, as it's only used when building the connection string, but I didn't want to introduce any breaking changes.

Also, I am pretty new to python and was having trouble adding my fork of the project as a package to an external test project. It would be nice if there were some guidance on that in the README. I tried following the instructions in both the poetry and pip documentation, but couldn't install from github as there is neither setup.py nor pyproject.toml in the root directory.

Create a new generic database subclass - DependencyFreeDbContainer Remove test that was testing sqlalchemy support for driver types Add tests for supported versions of Postgres Modify the `get_connection_url` convenience method to support a driverless url

Note that Postgres container no longer depends on SqlAlchemy Remove reference to unsupported version of postgres Show an example of using the `driver` parameter

Thanks for the contribution. Do you think we can achieve the same outcome by overriding _connect for the postgres container and moving the content of _verify_status into _connect?

Let's stick with 9.5 here. Pinning the version in the docs is to ensure that things don't accidentally break because one of the dependencies changed.

According to https://www.postgresql.org/support/versioning/, 9.5 is not supported anymore, the oldest supported version is 11. My suggestion would be to go with 15 and then have another 5 years until the docs have to be changed... :-)

Makes sense, let's bump to 15.

... so now 16 :-)

Let's remove the comment because the url will vary depending on the environment.

Hi,

any idea when this could be merged ? I’m using peewee as my orm and would love to get rid of that too.

This could also remove the hard dependency on psycopg2 which is slowly superseded by psycopg (v3). At least for us it's now the only package which pulls in the psycopg2 binary :-/

This actually needs to remove the dependencies on sqlalchemy and psycopg2

Hello friends, random person from the internet. If this feature was supported, it would be pretty nice. For those involved, if you could forward it (seems the developer needed some CI workflow manually triggered), it would be much appreciated 🙏

please add 15 and 16 and maybe remove 11 (not supported anymore)

When this would be expected to be merged?
Thanks!

@logicbomb Will you find time to get this running on top of the current main branch? If not: would you mind if I take this and rebase (or rework, if needed) and open it as a new PR (I will keep your handle as co-author on the commit)?

If not: would you mind if I take this and rebase (or rework, if needed) and open it as a new PR (I will keep your handle as co-author on the commit)?

Thanks for the activity, mate! I personally wouldn't mind you taking this on (my default attitude is "people are long gone" unless proven otherwise).

However, if I could ask for a tweak 😇 🥺

Make it not be in generic.py. These "db containers" assume a certain URL format, but is it worth it to have an entire class just for this? (I won't hide my distaste over DbContainer and why it is in core in the first place)
It's possible the same thing as for kafka, I would be happy with that approach, see fix(kafka): wait_for_logs in kafka container to reduce lib requirement #377

Make it not be in generic.py. These "db containers" assume a certain URL format, but is it worth it to have an entire class
just for this? (I won't hide my distaste over DbContainer and why it is in core in the first place)

So what's the alternative (in the order of my preference, best to lowest)?

Move it to a different file?
Make all the db container implement ´_create_connection_url()`?
Add a different layer, like singledispatch to turn the container into an URL?

It's possible the same thing as for kafka, I would be happy with that approach, see
#377

This is exactly what this PR does :-) It uses both logs and pg_isready to check instead of opening a connection. It's still missing that the dependency can be removed.

Hi Jen, Sorry for the late reply, I'm happy for you to take it over - i'm working on too many things atm. jt

…

On Mon, Mar 4, 2024 at 5:09 AM Jan Katins ***@***.***> wrote: Make it not be in generic.py. These "db containers" assume a certain URL format, but is it worth it to have an entire class just for this? (I won't hide my distaste over DbContainer and why it is in core in the first place) So what's the alternative (in the order of my preference, best to lowest)? - Move it to a different file? - Make all the db container implement ´_create_connection_url()`? - Add a different layer, like singledispatch to turn the container into an URL? It's possible the same thing as for kafka, I would be happy with that approach, see #377 <#377> This is exactly what this PR does :-) It uses both logs and pg_isready to check instead of opening a connection. It's still missing that the dependency can be removed. — Reply to this email directly, view it on GitHub <#340 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAASJJIOIKAHHWUN7HJNC4TYWRB6VAVCNFSM6AAAAAAXITOD2KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZWGIZDGMJYGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

I opened #445 for the rebased version.

closing in favor of #445 for now

…445) Updates the pg testcontainer implementation to not use (and not install) SQLAlchemy nor psycopg2. Closes: #340 Closes: #336 Closes: #320 --------- Co-authored-by: Jason Turim <jason@opscanvas.com>

…estcontainers#445) Updates the pg testcontainer implementation to not use (and not install) SQLAlchemy nor psycopg2. Closes: testcontainers#340 Closes: testcontainers#336 Closes: testcontainers#320 --------- Co-authored-by: Jason Turim <jason@opscanvas.com>

logicbomb mentioned this pull request Apr 23, 2023

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

Closed

logicbomb force-pushed the issue_336 branch from 6dcbd60 to 6c122d5 Compare April 23, 2023 15:51

logicbomb added 2 commits May 1, 2023 08:45

Update README.rst

c6bf6b9

Note that Postgres container no longer depends on SqlAlchemy Remove reference to unsupported version of postgres Show an example of using the `driver` parameter

logicbomb force-pushed the issue_336 branch from 6c122d5 to c6bf6b9 Compare May 1, 2023 12:45

tillahoffmann changed the title Issue 336 Remove sqlalchemy dependency from postgres connection check May 4, 2023

tillahoffmann linked an issue May 5, 2023 that may be closed by this pull request

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

Closed

tillahoffmann suggested changes May 5, 2023

View reviewed changes

Respond to comments

4713d28

tillahoffmann added the 📦 package: postgres label May 9, 2023

tillahoffmann mentioned this pull request May 11, 2023

Make the kafka package dependency optional to be able to use confluent_kafka #351

Closed

github-actions Bot added the 👀 requires attention label Aug 29, 2023

jankatins suggested changes Oct 19, 2023

View reviewed changes

jankatins mentioned this pull request Oct 19, 2023

add postgres sqlalchemy asyncio support #359

Closed

jankatins reviewed Jan 23, 2024

View reviewed changes

jankatins mentioned this pull request Mar 6, 2024

fix(postgres): Remove SqlAlchemy dependency from postgres container #445

Merged

alexanderankin closed this Mar 9, 2024

-                  >>> with PostgresContainer("postgres:9.5") as postgres:
-                  ...     engine = sqlalchemy.create_engine(postgres.get_connection_url())
+                  >>> with PostgresContainer("postgres:latest") as postgres:
+                  ...     psql_url = postgres.get_connection_url() # postgresql+psycopg2://test:test@localhost:61472/test

-                          for row in result:
-                              assert row[0].lower().startswith("postgresql 9.5")
+                  # https://www.postgresql.org/support/versioning/
+                  supported_versions = ["11", "12", "13", "14", "latest"]

                   >>> from testcontainers.postgres import PostgresContainer
                   >>> import sqlalchemy
-                  >>> with PostgresContainer("postgres:9.5") as postgres:

Remove sqlalchemy dependency from postgres connection check by logicbomb · Pull Request #340 · testcontainers/testcontainers-python · GitHub

Uh oh!

Remove sqlalchemy dependency from postgres connection check - #340

Remove sqlalchemy dependency from postgres connection check#340logicbomb wants to merge 3 commits intotestcontainers:maintestcontainers/testcontainers-python:mainfrom logicbomb:issue_336logicbomb/testcontainers-python:issue_336Copy head branch name to clipboard

Conversation

logicbomb commented Apr 23, 2023

Uh oh!

tillahoffmann left a comment

Choose a reason for hiding this comment

Uh oh!

tillahoffmann May 4, 2023

Choose a reason for hiding this comment

Uh oh!

jankatins May 11, 2023

Choose a reason for hiding this comment

Uh oh!

tillahoffmann May 11, 2023

Choose a reason for hiding this comment

Uh oh!

jankatins Jan 23, 2024

Choose a reason for hiding this comment

Uh oh!

tillahoffmann May 4, 2023

Choose a reason for hiding this comment

Uh oh!

nicos68 commented Aug 29, 2023

Uh oh!

jankatins commented Oct 19, 2023

Uh oh!

jankatins left a comment

Choose a reason for hiding this comment

Uh oh!

jamesbraza commented Nov 11, 2023

Uh oh!

jankatins Jan 23, 2024

Choose a reason for hiding this comment

Uh oh!

d-vuletas-levi9-com commented Feb 20, 2024

Uh oh!

jankatins commented Feb 28, 2024

Uh oh!

totallyzen commented Mar 4, 2024

Uh oh!

jankatins commented Mar 4, 2024

Uh oh!

logicbomb commented Mar 4, 2024 via email

Uh oh!

jankatins commented Mar 6, 2024

Uh oh!

alexanderankin commented Mar 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Remove sqlalchemy dependency from postgres connection check#340
logicbomb wants to merge 3 commits into
testcontainers:mainfrom
logicbomb:issue_336