feat(core): Wait strategies foundation #838
Conversation
|
Notes: Mypy errors got crazy and circular, so I skipped them for now to focus on getting the wait strategies up without the massive changes caused by bringing those core files inline with mypy. Also the comments are representing some strategies not included here, but I do have working versions ready to be brought in 🥳 |
terry-docker
changed the title
|
I assume mypy needs all the good stuff on #810 🙂 |
terry-docker
force-pushed
the
wait-strategy-foundation
branch
from
1bc760a to
4bf23d8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #838 +/- ##
=======================================
Coverage ? 81.79%
=======================================
Files ? 14
Lines ? 890
Branches ? 140
=======================================
Hits ? 728
Misses ? 127
Partials ? 35 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
if i can merge 810 and rebase this on top of that that would be ideal. i hope to be done with #810 on sunday or monday and this can follow shortly after |
|
Sure, I can add the missing new mypy types on this PR too. |
rhoban13
left a comment
rhoban13
left a comment
There was a problem hiding this comment.
This is great - thanks!
|
|
||
|
|
||
| def wait_container_is_ready(*transient_exceptions) -> Callable: | ||
| class WaitStrategyTarget(Protocol): |
There was a problem hiding this comment.
I wonder if WaitStrategyTarget & WaitStrategy might be split into their own module - somewhat separating it from the now deprecated functions in here. That might help simplify when time comes for its eventual removal.
There was a problem hiding this comment.
If we can avoid changing files it preserves git history better. If this file was called wait or waiting it would have been better but I don't see removing utils or reorganizing code a super high priority
There was a problem hiding this comment.
I was on the fence about this, they're needed for the backwards compatibility but I honestly don't have strong feelings about where to have WaitStrategyTarget and WaitStrategy live
|
made some progress on conflicts here today |
|
if i cant figure out how to make this go away, i might just disable code coverage at all - it is supposed to be useful tool not a procrustean bed (https://www.dictionary.com/browse/procrustean-bed) 
im trying to copy code out of the ui and its not working because its in the way. </vent> |
alexanderankin
force-pushed
the
wait-strategy-foundation
branch
from
4bf23d8 to
ee4f335
Compare
alexanderankin
force-pushed
the
wait-strategy-foundation
branch
2 times, most recently
from
dd156fa to
3e6dd71
Compare
|
@terry-docker if you want to edit the git history that is up to you or however you want to merge this - currently it does not give you credit but it is mergeable - i will take a look when i have time |
terry-docker
force-pushed
the
wait-strategy-foundation
branch
from
3e6dd71 to
2de97d9
Compare
| # from pathlib import Path | ||
| # from typing import Final, Any, Generator | ||
| # | ||
| # import pytest |
There was a problem hiding this comment.
What happened here?
Introducing a new feature and not being able to keep old tests running is one thing, but just commenting them out , in order that the complete file is changed and collapsed in review?
That is really bad style especially while "claiming"
No breaking changes - all existing code continues working
If tests are not working at least have the courage to add @pytest.skip(reason="need rework after wait utils change")
Docker in Docker (Or docker out of docker) is really important for CI/CD and a standing issue (see pinned #517).
I actually did the work to add tests to cover more example of DinD/DooD and seeing this tests just commented out, as people didn't want to bother with is hurting.
I am aware that is is probably an accident while there were Problems to get the tests locally running...
In this case one could add markers and exclude them when running pytest locally
There was a problem hiding this comment.
hmmmm, I must have missed this. ultimately we definitely need the dind test back,
There was a problem hiding this comment.
Ok, lets try this one more time when I am fully awake. @CarliJoy I take full responsibility here.
When Terry volunteered her time at work to work on this project, I was asked to provide some direction.
The direction I provided was try to contribute docs and waiting, and if anything breaks, don't try to worry about it too much because I will handle it. (Otherwise, there would be serious risk of not getting either thing done - all time going to maintenance instead of new feature development).
My failure to create a ticket or reminder to myself to restore the unit test you contributed was perhaps irresponsible but what I want to apologize for truly is any disrespect to you. Some of my maintenance conduct in this project does not always respect contributor time because of the such limited time budget this project has, and I will do better here to keep up, including with larger PRs.
There was a problem hiding this comment.
No worries — mistakes happen 🤗
Thank you for your work and for the time you invest here. This is also free time you are giving to the project, and that is something I genuinely appreciate. Even if we have different roles, your time and effort still matters as much.
Maintaining a project is simply not easy. Having final responsibility means not only making technical decisions, but also constantly deciding where to spend limited time and attention. Those trade-offs are unavoidable, and with them come decisions that not everyone will agree with - sometimes not even our future self ;-)
6 participants
Aligns testcontainers-python with the testcontainers community standard wait strategy pattern used across Java, Go, and other implementations. This provides a consistent developer experience and better maintainability.
Examples
From:
To:
Backward Compatibility
No breaking changes - all existing code continues working
Deprecation warnings added to wait_for_logs() and @wait_container_is_ready
Clear migration path provided in warning messages
New:
core/testcontainers/core/wait_strategies.py - Strategy implementations
core/tests/test_wait_strategies*.py - Comprehensive test coverage
Modified:
core/testcontainers/core/container.py - Added waiting_for() method
core/testcontainers/compose/compose.py - Added compose wait strategy support
core/testcontainers/core/waiting_utils.py - Base classes and protocol
Future Strategies to quickly follow
Foundation enables community-standard wait strategies:
HttpWaitStrategy, HealthcheckWaitStrategy, PortWaitStrategy, CompositeWaitStrategy
Testing
Unit tests with parameterized scenarios
Integration tests with real Docker containers
Protocol compliance verification
Backward compatibility validation