Skip to content
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

Unable to provide timeout for waiting #247

Open
ctron opened this issue Jan 26, 2021 · 6 comments
Open

Unable to provide timeout for waiting #247

ctron opened this issue Jan 26, 2021 · 6 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@ctron
Copy link
Contributor

ctron commented Jan 26, 2021

When using "wait for" there seems to be no timeout.

While you can add a timeout to tests in general, it would make more sense for me to put a much shorter timeout for the startup of a container.

@thomaseizinger
Copy link
Collaborator

When using "wait for" there seems to be no timeout.

Correct.

Generally, we try to ship testcontainers in a way that works out-of-the-box, i.e. with pinned versions of containers and statically-encoded parameters and env variables. Green tests on our side should therefore ensure that this particular image will always work for the end user.

What is the usecase in which you would have needed a timeout?

While it would certainly be possible to pass or configure one, I'd be more interested in making it harder / impossible to use a docker image through testcontainers in a way that it wouldn't successfully start up.

@ctron
Copy link
Contributor Author

ctron commented Jan 27, 2021

What is the usecase in which you would have needed a timeout?

The issue I ran into is that it looks like Postgres containers fail to start up properly on GitHub actions. So the expected line doesn't show up in the logs, and the whole job is stuck.

While it would certainly be possible to pass or configure one, I'd be more interested in making it harder / impossible to use a docker image through testcontainers in a way that it wouldn't successfully start up.

I think there will always be a case you didn't think about. Allowing to put in a timeout make total sense to me.

@thomaseizinger
Copy link
Collaborator

That makes sense, thanks for sharing!

@thomaseizinger
Copy link
Collaborator

Do you think we should have a fixed timeout or something that is configurable per container?

I am leaning towards a global one that can maybe be overridden through an env variable to keep the images themselves simple.

@ctron
Copy link
Contributor Author

ctron commented Feb 2, 2021

I am leaning towards a global one that can maybe be overridden through an env variable to keep the images themselves simple.

Sounds reasonable to me. If additional requirements come up, it should be easy to extend. But it should also cover most cases already.

@thomaseizinger
Copy link
Collaborator

I propose a default timeout of 1 minute for now.

If we want to make this configurable, #259 should be a good foundation for that. I am thinking of something like:

TESTCONTAINERS=ready_timeout=2m,keep

@thomaseizinger thomaseizinger added the help wanted Extra attention is needed label Feb 3, 2021
@DDtKey DDtKey self-assigned this May 20, 2024
@DDtKey DDtKey added enhancement New feature or request and removed help wanted Extra attention is needed labels May 20, 2024
@DDtKey DDtKey added this to the 0.17.0 milestone May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants