-
Notifications
You must be signed in to change notification settings - Fork 60
Add support for extended logging in DbConfig #1003
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: bar haim <[email protected]>
Signed-off-by: bar haim <[email protected]>
5e85a4c
to
f7256e3
Compare
Signed-off-by: bar haim <[email protected]>
cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) | ||
require.NoError(t, err) | ||
|
||
ctx := context.Background() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use t.Context()
as this allows the test harness to cancel the context and all probably all consumers of the context.
// Check extended logging parameters based on expectation | ||
cmd := inspect.Config.Cmd | ||
if expectExtendedLogging { | ||
assert.Contains(t, cmd, "log_destination=stderr") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if there is a particular reason to use assert
over require
here. The difference is that assert
does not call FailNow
.
|
||
// Add extended logging configuration if enabled | ||
if c.ExtendedLogging { | ||
containerConfig.Cmd = []string{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ExtendedLogging
toggle probably makes sense for many cases and I can imagine that when debugging with postgres, sometimes more flexibility is needed when investigating an odd issue.
I think it would be nice to allow the Cmd
be overwritten via env vars to let the user inject any postgres command / env vars via testing without touching this code all the time.
No description provided.