fix: remove non-transient logs on missing artifact-repositories configmap

[thor]

Fixes #9085.

Motivation

Remove high levels of error messages that aren't errors. Reduce additional observability costs associated with running Argo Workflows, although negligble.

Clarify the difference between side-effect and non-side-effect functions in utility module.

Modifications

Renames IsTransientErr to LogIsTransientErr, which is easily undoable if it's more noise than signal.

Introduces a LogOnlyTransientErr, which only logs errors if they're transient. Suitable for a use-case such as trying to fetch an object that doesn't exist when it's optional.

Verification

No verification performed due to the (deceptive) simplicity of the change.

[Joibel]

The two new function names LogIsTransientError and LogOnlyIsTransient error imply things about their purpose which aren't really their primary use (logging, not a test).

They also don't immediately make sense - two functions called LogDogName and LogOnlyDogName I wouldn't know which to call to log the dogs name. The function names you've created make sense if you understand the history. The Is part of their name helps, but...

I'd suggest either calling them IsTransientErr and IsTransientErrVerbose, or my slight preference would be IsTransientErrQuiet and IsTransientErr.

Naming things is hard.

[thor]

The two new function names LogIsTransientError and LogOnlyIsTransient error imply things about their purpose which aren't really their primary use (logging, not a test).

They also don't immediately make sense - two functions called LogDogName and LogOnlyDogName I wouldn't know which to call to log the dogs name. The function names you've created make sense if you understand the history. The Is part of their name helps, but...
[...]
Naming things is hard.

Thanks for the review, @Joibel, much appreciated. I solidly agree, naming things is hard.

I'd suggest either calling them IsTransientErr and IsTransientErrVerbose, or my slight preference would be IsTransientErrQuiet and IsTransientErr.

Considering that the specific scenario wants to ignore non-transient errors only, do I understand you correctly that IsTransientErrQuiet seems appropriate although it won't be quiet, just less verbose?

EDIT: I've assumed that you indeed meant IsTransientErrQuiet was appropriate although it's not fully quiet, re: naming things, and simplified the PR to only switch the lookup of the configuration map artifact-repositories to IsTransientErrQuiet.

[Joibel]

EDIT: I've assumed that you indeed meant IsTransientErrQuiet was appropriate although it's not fully quiet, re: naming things, and simplified the PR to only switch the lookup of the configuration map artifact-repositories to IsTransientErrQuiet.

It's Quieter, but in the interests of sane shorter names it felt better to just call it Quiet.

Thanks, for doing this.

[thor]

@Joibel I don't think the minimal E2E test that's failing here is relevant to the code change. Is that something you could confirm, and if so, merge the PR?

[Joibel]

I'll re-run them and see if they pass, they look like a flakey test but I'd like to be sure.