Skip to content

add support async postgres driver - #320

Closed
fourteekey wants to merge 6 commits into
testcontainers:mainfrom
fourteekey:master
Closed

add support async postgres driver#320
fourteekey wants to merge 6 commits into
testcontainers:mainfrom
fourteekey:master

Conversation

@fourteekey

@fourteekeyfourteekey commented Mar 13, 2023

Copy link
Copy Markdown

No description provided.

@fourteekeyfourteekey mentioned this pull request Mar 13, 2023
Open
@pffijtpffijt linked an issue Mar 15, 2023 that may be closed by this pull request
Creating a Postgres contrainer with an async driver fails #263
Open
pffijt
pffijt reviewed Mar 15, 2023
View reviewed changes
Comment threadcore/testcontainers/core/generic.py Outdated
@fourteekey
fourteekey requested a review from pffijtMarch 16, 2023 11:20
@fourteekey

fourteekey commented Apr 5, 2023

Copy link
Copy Markdown
Author

@pffijt
Do you have any questions or comments about commits?

tillahoffmann
tillahoffmann suggested changes Apr 11, 2023
View reviewed changes

@tillahoffmanntillahoffmann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. Added a few inline questions.

Comment threadpostgres/setup.py Outdated
Comment threadpostgres/testcontainers/postgres/__init__.py Outdated
Comment threadpostgres/testcontainers/postgres/__init__.py
@wait_container_is_ready(*ADDITIONAL_TRANSIENT_ERRORS)
def _connect(self) -> None:
import sqlalchemy
engine = sqlalchemy.create_engine(self.get_connection_url(driver=self.DEFAULT_DRIVER))

@tillahoffmanntillahoffmannApr 11, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just use the super call here? It looks like the method body is the same as in the parent class (except for the driver argument which is already handled in the updated get_connection_url).

@tillahoffmanntillahoffmannMay 16, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fourteekey, could you please address this question?

@tillahoffmann

tillahoffmann commented May 5, 2023

Copy link
Copy Markdown
Contributor

Unresolved some of the outstanding questions, @fourteekey. We'll also have to regenerate the lock files in requirements.txt. See step three of https://github.com/testcontainers/testcontainers-python#contributing-a-new-feature for details.

@tillahoffmanntillahoffmann mentioned this pull request May 5, 2023
Merged
@fourteekey

fourteekey commented May 16, 2023

Copy link
Copy Markdown
Author

@tillahoffmann

All questions have been resolved.
I can't regenerate the lock files without approve, I got error: RuntimeError: Could not find a workflow. Has the GitHub Action run completed? If youare a first-time contributor, a contributor has to approve your changesbefore Actions can run.

@Evgen4567Evgen4567 mentioned this pull request May 26, 2023
Closed
@alexanderankin

alexanderankin commented Mar 9, 2024

Copy link
Copy Markdown
Member

closing in favor of #445 because i believe it will handle this use case, let me know if this is not the case

@jankatinsjankatins mentioned this pull request Mar 10, 2024
Merged
alexanderankin pushed a commit that referenced this pull request Mar 10, 2024
@jankatins@logicbomb
feat(postgres): Remove SqlAlchemy dependency from postgres container (#…
f30eb1d
…445)
Updates the pg testcontainer implementation to not use (and not install)
SQLAlchemy nor psycopg2.
Closes: #340Closes: #336Closes: #320
---------
Co-authored-by: Jason Turim <jason@opscanvas.com>
alexanderankin pushed a commit that referenced this pull request Mar 11, 2024
@jankatins@logicbomb
fix(postgres): Remove SqlAlchemy dependency from postgres container (#…
940167e
…445)
Updates the pg testcontainer implementation to not use (and not install)
SQLAlchemy nor psycopg2.
Closes: #340Closes: #336Closes: #320
---------
Co-authored-by: Jason Turim <jason@opscanvas.com>
bearrito pushed a commit to bearrito/testcontainers-python that referenced this pull request Mar 30, 2024
@jankatins@logicbomb@bearrito
feat(postgres): Remove SqlAlchemy dependency from postgres container (t…
0f79144
…estcontainers#445)
Updates the pg testcontainer implementation to not use (and not install)
SQLAlchemy nor psycopg2.
Closes: testcontainers#340Closes: testcontainers#336Closes: testcontainers#320
---------
Co-authored-by: Jason Turim <jason@opscanvas.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pffijtpffijtAwaiting requested review from pffijt
1 more reviewer
@tillahoffmanntillahoffmanntillahoffmann requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

🚀 enhancement📦 package: postgres

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Creating a Postgres contrainer with an async driver fails

4 participants

@fourteekey@tillahoffmann@alexanderankin@pffijt