-
-
Notifications
You must be signed in to change notification settings - Fork 277
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
[Bug]: cannot replace command (or any other enumerable) #1227
Comments
Hi, at this point, this is intentional. Modules are pre-configured, opinionated, and follow best practices (configurations) driven by the community. The builder API is "immutable" to support A/B testing, it is currently not possible to override (remove) configurations for most elements that rely on lists or dictionaries (due to the way the Supporting a feature that allows developers to entirely override the configuration would likely be a good enhancement, but it should be implemented explicitly. There are many variations that could potentially break the pre-configured module configuration, and developers should be aware that this could easily cause issues. However, if a module's configuration does not suit your use case, you can always fall back to the generic builder and configure it yourself. |
Thanks @HofmeisterAn ...
Can you please clarify this? Do you mean to extend |
Yes, this is what I meant.
I understand, but it is difficult to provide the desired flexibility, supporting all kinds of configurations and maintaining reliability at the same time. |
Got it. Just for the record, I decided to use reflection to reduce the amount of code. It's in my test fixture anyway, so not a huge deal. type KeycloakBuilder with
member this.WithOptimizedStartup() =
let keycloakConfiguration =
this
.GetType()
.GetProperty("DockerResourceConfiguration", BindingFlags.Instance ||| BindingFlags.NonPublic ||| BindingFlags.DeclaredOnly)
.GetValue(this)
:?> KeycloakConfiguration
if keycloakConfiguration = null then
failwith "Failed to resolve DockerResourceConfiguration property on KeycloakConfiguration"
let commandField =
keycloakConfiguration
.GetType()
.BaseType
.GetFields(BindingFlags.Instance ||| BindingFlags.NonPublic)
|> Array.tryFind _.Name.StartsWith("<Command>")
|> Option.toObj
if commandField = null then
failwith "Failed to resolve backing field for ContainerConfiguration.Command property"
commandField.SetValue(keycloakConfiguration, [ "start"; "--optimized" ] |> box)
this |
If you like, we can keep the issue open. IMO, it is a valid feature request to be able to override module configurations. With the current design, overriding some parts is very tricky, especially when trying to prevent developers from making misconfigurations. However, we might find a better design that allows developers to properly override module configurations while also creating awareness that with the overridden values, wait strategies, connection strings, etc., may not work as expected anymore. |
Sure thing, thanks |
Testcontainers version
3.9.0
Using the latest Testcontainers version?
Yes
Host OS
Windows
Host arch
x86
.NET version
8.0.303
Docker version
Client: Version: 27.0.3 API version: 1.46 Go version: go1.21.11 Git commit: 7d4bcd8 Built: Sat Jun 29 00:03:32 2024 OS/Arch: windows/amd64 Context: desktop-linux Server: Docker Desktop 4.32.0 (157355) Engine: Version: 27.0.3 API version: 1.46 (minimum version 1.24) Go version: go1.21.11 Git commit: 662f78c Built: Sat Jun 29 00:02:50 2024 OS/Arch: linux/amd64 Experimental: false containerd: Version: 1.7.18 GitCommit: ae71819c4f5e67bb4d5ae76a6b735f29cc25774e runc: Version: 1.7.18 GitCommit: v1.1.13-0-g58aa920 docker-init: Version: 0.19.0 GitCommit: de40ad0
Docker info
What happened?
When building a container - any container, but in my case Keycloak - specifying a different command results in merging. If the
ContainerBuilder
already defines a "default" command, there's seemingly no way to replace that command completely. For example:This will result in an attempt to run the command
start-dev start
rather than the desiredstart
becauseKeycloakBuilder.Init
already invokes.WithCommand("start-dev")
. Moreover, I cannot find a way to supplant the default like you can with non-enumerable values. The easiest thing to do for a quick workaround would be to inheritKeycloakBuilder
and overrideInit
so that it doesn't invokeWithCommand
, butKeycloakBuilder
issealed
.Relevant log output
No response
Additional information
No response
The text was updated successfully, but these errors were encountered: