You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The original deck design of these libraries had very limited logging. It did not use a logging library, and instead uses a basic wrapper around fmt.Println() to output directly to stdout/stderr. It has a basic global on/off toggle at the library level, though some consumers further add their own wrappers to simulate additional log levels.
To better make these packages a library, we should instead use a logging library or return rich structures to allow consumers to choose their desired log level and behavior. Library features that currently use cprint should instead accept a logger from their instantiator and send log lines to it an appropriate verbosity level, or should eschew logging altogether in favor of returning data that callers can handle as they wish.
There are currently 3 uses of cprint in this library:
The diff packages prints updated resource lines and possibly JSON diffs.
The types basic-auth handler has a warning it prints when you attempt to use basic-auths with deck.
The util package can print a warning called from file.Builder.build() if routes use regex syntax incompatible with their version.
The nature of these uses push me towards saying using returned values rather than accepting a logger. The full JSON diffs offered by diff do not readily work with most logging systems, since they're ideally printed over multiple lines for human consumption. These probably require handling (outside the normal logging system) downstream to present effectively. Continuing to print these to stderr makes sense for the deck CLI, but not so much for KIC--KIC does currently dump these to log output at higher verbosity, but it'd probably make more sense to place these in a separate file accessible via the config diagnostic API.
The warning print should probably be handled via initial scans of the input YAML. The deck CLI can scan input to see if basic-auths are present at all and print that warning (though weirdly as I read it we currently only print these on delete actions? idk why--this feature maybe needs further review), and can scan for regexes that don't match the Kong version similarly. KIC simply does not generate the wrong regex type, and has no issues using basic-auth that I know of.
Acceptance:
go-database-reconciler no longer provides the cprint package. Any downstream consumers (likely only deck) provide their own instead if its functionality is desired.
Functionality that currently uses cprint instead returns rich values that it would have previously formatted and output itself.
The text was updated successfully, but these errors were encountered:
The original deck design of these libraries had very limited logging. It did not use a logging library, and instead uses a basic wrapper around
fmt.Println()
to output directly to stdout/stderr. It has a basic global on/off toggle at the library level, though some consumers further add their own wrappers to simulate additional log levels.To better make these packages a library, we should instead use a logging library or return rich structures to allow consumers to choose their desired log level and behavior. Library features that currently use
cprint
should instead accept a logger from their instantiator and send log lines to it an appropriate verbosity level, or should eschew logging altogether in favor of returning data that callers can handle as they wish.There are currently 3 uses of
cprint
in this library:diff
packages prints updated resource lines and possibly JSON diffs.types
basic-auth handler has a warning it prints when you attempt to use basic-auths with deck.util
package can print a warning called fromfile.Builder.build()
if routes use regex syntax incompatible with their version.The nature of these uses push me towards saying using returned values rather than accepting a logger. The full JSON diffs offered by
diff
do not readily work with most logging systems, since they're ideally printed over multiple lines for human consumption. These probably require handling (outside the normal logging system) downstream to present effectively. Continuing to print these to stderr makes sense for the deck CLI, but not so much for KIC--KIC does currently dump these to log output at higher verbosity, but it'd probably make more sense to place these in a separate file accessible via the config diagnostic API.The warning print should probably be handled via initial scans of the input YAML. The deck CLI can scan input to see if basic-auths are present at all and print that warning (though weirdly as I read it we currently only print these on delete actions? idk why--this feature maybe needs further review), and can scan for regexes that don't match the Kong version similarly. KIC simply does not generate the wrong regex type, and has no issues using basic-auth that I know of.
Acceptance:
cprint
package. Any downstream consumers (likely only deck) provide their own instead if its functionality is desired.The text was updated successfully, but these errors were encountered: