Put HttpWaitStrategy behind a feature flag

[rukai]

Most containers will output a log on stdout when they are ready to be used, so needing to send an http request is not always needed. Putting it behind a feature flag would allow users who dont need this to skip the large reqwest crate and all its deps.

If desired this http_wait feature could be a default feature to make it conveniently enabled by default for users who need it.

I'm happy to make a PR if this is wanted.

[DDtKey]

Hi @rukai 👋

This certainly sounds reasonable. But in fact, reqwest does't increase compilation time that much. Almost all of its dependencies are common with the essential underlying Docker client - bollard.

In fact, this was the only reason why feature-gate was not used for the http strategy initially (although, might be a mistake).

However, this still includes reqwest itself, and these crates may use different major versions of their deps.

So the feature definitely makes sense and is cargo-way too.

The only question: do we want to make a breaking change for this or should keep enabled by default(for a while, at least)? We can preserve compatibility and allow the extra dep to be disabled using default-features=false.

Although, changing this later may be confusing.

[rukai]

But in fact, reqwest does't increase compilation time that much.

I noticed many Cargo.lock entries dissapear after removing reqwest, although admittedly didnt pay attention to which dependencies they were or how big they were.

The only question: do we want to make a breaking change for this

IMO moving existing functionality behind a default feature flag is a breaking change, since anyone who uses default-features = false and uses the feature would be broken.

[DDtKey]

IMO moving existing functionality behind a default feature flag is a breaking change, since anyone who uses default-features = false and uses the feature would be broken.

Agree, that's breaking change anyway.

So let's go the direct way - make it optional dependency. Hopefully it won't affect many users, because the main driver was testcontainers-modules (clickhouse module, particularly) And module will be updated transparently (still breaking change though)

I noticed many Cargo.lock entries dissapear after removing reqwest, although admittedly didnt pay attention to which dependencies they were or how big they were

Yeah, I mostly meant big ones - hyper, http, serde and etc. But it's not that important. Still makes sense