feat: enable Provider verbose logging for statements #851
Conversation
Currently, `Provider` with `WithVerbose` is not honored by the provider. Simplest approach was to update `runGo` and `runSQL` to make these functions Pointer Receiver of `Provider` in order to have a provider `Config` reachable. Another approach would be to overload the `Context` with `ContextWithValue` and set `cfg`, which is dirtier IMHO because it leaves us with an interface casting Reusing the global `verbose` variable and verbose logging is not a good idea either because this is not a per provider behaviour.
| @@ -437,6 +438,9 @@ func runSQL(ctx context.Context, db database.DBTxConn, m *Migration, direction b | |||
| statements = m.sql.Down | |||
| } | |||
| for _, stmt := range statements { | |||
| if p.cfg.verbose { | |||
| p.cfg.logger.Printf("Excuting statement: %s", stmt) | |||
There was a problem hiding this comment.
I'm not sure I want these in my logs when enabling verbose logging. I.e., I care about the final output of the migration, and less about the individual statements. This almost feels more like debugging.
There was a problem hiding this comment.
Tricky then, because this is exactly the same as
Line 38 in 512a496
Here, I've just added the same behaviour for the provider as the goose usage without provider.
This almost feels more like debugging.
Not really, since this is executed in an ideal world under some CD tooling, by any team, if would be nice to ensure that we have enough logs to assess a situation.
We had an issue where adding 40 columns, only 14 where added. We discovered why, but it would have helped to have the Goose Executing Statement log.
For me, this is verbose logging behaviour that is supposed to be handled the right way by the provider. If, what is WithVerbose used for ?
There was a problem hiding this comment.
Okay fair, I think I can get behind this.
|
I'd be happy to further discuss this. tia. |
mfridman
changed the title
Currently,
ProviderwithWithVerboseis not honored by the provider.Simplest approach was to update
runGoandrunSQLto make these functions Pointer Receiver ofProviderin order to have a providerConfigreachable.Another approach would be to overload the
ContextwithContextWithValueand setcfg, which is dirtier IMHO because it leaves us with an interface casting.Reusing the global
verbosevariable and verbose logging is not a good idea either because this is not a per provider behaviour and still the logger won't be reachable.