Don't re-use reaper if it failed to initialize#258
Don't re-use reaper if it failed to initialize#258kernle32dll wants to merge 3 commits intotestcontainers:mainfrom
Conversation
|
|
||
| c, err := provider.RunContainer(ctx, req) | ||
| if err != nil { | ||
| reaper = nil |
There was a problem hiding this comment.
Hey @kernle32dll, thanks for taking the effort of reporting this!
I see this PR would work although I'm concerned about its maintainability if new code conditions are added in the future.
How would you see delaying the creation of the Reaper struct instance (L51) until the container and the endpoint are created (right before the old L87)? I'd see the code more maintainable in this case. wdyt?
|
@kernle32dll are you still interested in this PR? If not, I'd like to label it as |
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
With merging #782, I believe this PR became obsolete and can be closed? |
|
I believe so. I'm closing it but feel free to reopen if it persists, thanks! |
3 participants
If the reaper fails to initialize, it leaves testcontainers in a broken state. To be precise, the
Endpointvariable is never properly initialized, which causes subsequent usages of testcontainers to fail withfailed to create container: connecting to reaper failed: Connecting to Ryuk on failed: dial tcp: missing address.