fix: remove configuration reading from logger initialization #2661
Conversation
Due to the addition of a new init function for the logger initialization and Ryuk disable message, the environment variables were reading as an init function side effect. This caused variables set on the code using the `os` package does not read property. It removes the reading for the environment variables from the logger initialization, and prints the message in a more appropriate location.
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -18,6 +18,17 @@ type TestcontainersConfig struct { | |||
| // of the TestcontainersConfig struct | |||
| func ReadConfig() TestcontainersConfig { | |||
| cfg := config.Read() | |||
|
|
|||
| if cfg.RyukDisabled { | |||
There was a problem hiding this comment.
Will this be printed anytime that a user reads the config? As this method is public API, it would be a bit noisy seeing that message that much.
There was a problem hiding this comment.
I, personally, don't see a problem, as this is a kind of warning message. What do you think could be a better approach?
|
Relates #2636 |
|
@mdelapenya we are affected by this bug and this PR would solve our issues. What's missing for you to approve and merge? |
@sum-elier I returned from summer holidays yesterday. I'm keeping up with the email, notifications and stuff. Will go back to it asap. OTOH I'd like to discuss more in #2661 (comment), as I'd prefer not printing out that message for each call to config.Read |
|
Didn't spot this and created #2725 which just removes the message totally, as its opt in I would suggest the users knows what they are doing and doesn't want a warning about a conscious decision they have made. |
|
I think we can close this one, as #2725 has been merged. @henrybarreto could you verify it with the current HEAD in main? In the meantime I'm closing this one, so please reopen if needed. Thank you! |
What does this PR do?
It removes the reading for the environment variables from the logger initialization, and prints the message in a more appropriate location.
Why is it important?
Due to the addition of a new init function for the logger initialization and Ryuk disable message, the environment variables were reading as an init function side effect. This caused variables set on the code using the package does not read property.