Skip to content

enhancement: Easier configuration of the postgres command #9629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

KyleAure
Copy link
Contributor

@KyleAure KyleAure commented Dec 12, 2024

Currently, the PostgreSQLContainer configures fync=off during initialization of the container class.
This setting can be accidentally overwritten by using the withCommand() or setCommand() methods.
It would be more extensible to keep a list of configuration options for the postgres command, and instead configure a command during the configure lifecycle step.

Opening this PR because this is how I use the PostgreSQL container and thought maybe others might find this useful.
If there is no interest feel free to close this PR.

@KyleAure KyleAure requested a review from a team as a code owner December 12, 2024 16:22
@richardfearn
Copy link

Slightly related to #938

@richardfearn
Copy link

I would also love a way to do this via the JDBC URL. The 'best' option I've come up with so far is a new TC_ (container) parameter for a list of comma-separated, URL-encoded, additional command-line options to add to the command. The configure method in PostgreSQLContainer can then split/decode them and append them to the command.

The URL could look something like this, for example:

jdbc:tc:postgresql:15.10:///integration_tests?TC_INITSCRIPT=initPostgres.sql&TC_COMMAND_EXTRA=-c,log_statement%3Dall

@KyleAure
Copy link
Contributor Author

@richardfearn Thanks for posting an alternative option for others.
Unfortunately, for many of my complex test cases I cannot use the Testcontainer based JDBC URL.

@eddumelendez
Copy link
Member

Thanks for your contribution, @KyleAure. I think would be nice to add this to the GenericContainer instead. So, every implementation get benefit of it.

@richardfearn for more complex scenarios it is recommended to move from Testcontainers JDBC URL to PostgreSQLContainers.

@KyleAure
Copy link
Contributor Author

@eddumelendez

I think would be nice to add this to the GenericContainer instead. So, every implementation get benefit of it.

I believe that the GenericContainer already has an option for this which would be withCommand.
The reason we an additional configuration method in Postgres is because we need to configure both a default option fsync=off AND to allow users of the container object the ability to add additional command options.

I don't know how much value other containers would have for such a method, and any container that would want that functionality would still need to itself define the default command, and any default options.
Therefore, adding this to the GenericContainer would still require refactoring of individual container classes to be able to implement the use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants