Add flag --log-credentials-errors #57
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dcermak
force-pushed
the
dont-fail-as-zypp-plugin
branch
from
3c54bf4 to
fd1fcf9
Compare
jlausuch
approved these changes
Jun 14, 2022
dirkmueller
reviewed
Jun 15, 2022
Member
|
As a user/customer, the behavior I would expect is
This patch is putting the burden of finding out which case it is on the user, and it changes the default to the second, unregistered option. Following the 'supported customers first' principle I am wondering if we can do something to make the first option also as convenient. can we keep the default of the behavior, but handle only the case of "BCI repository available and no registration code provided" silently instead? Detecting whether "BCI repository available" could be done by calling this via a new name that enables the new behavior, so existing invocations remain unaffected. |
dcermak
force-pushed
the
dont-fail-as-zypp-plugin
branch
from
fd1fcf9 to
2f1a83c
Compare
dcermak
changed the title
dcermak
requested review from
djoreilly and
jlausuch
and removed request for
djoreilly
dirkmueller
reviewed
Nov 8, 2022
dirkmueller
reviewed
Nov 8, 2022
dirkmueller
approved these changes
Nov 8, 2022
Member
dirkmueller
left a comment
dirkmueller
left a comment
There was a problem hiding this comment.
LGTM. thank you for addressing the previous feedback!
I'm highly suspicous that tests do not need adjustment, but lets not look there
Currently container-suseconnect will print errors to the console when it fails to obtain the SLE credentials. This is however causing UX issues in the SLE BCI containers, which work perfectly fine without a SLE subscription. Thus a new flag is introduced: `--log-credentials-errors`. When this flag is set or if the environment variable `CONTAINER_SUSECONNECT_LOG_CREDENTIALS_ERR` is set, then `container-suseconnect` will continue printing errors as it did, but the default is now that errors with obtaining the credentials will be silently swallowed so that users of SLE BCI don't get confused by this error message. This fixes SUSE#62
dcermak
force-pushed
the
dont-fail-as-zypp-plugin
branch
from
2f1a83c to
1173fbd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently container-suseconnect will print errors to the console when it fails
to obtain the SLE credentials. This is however causing UX issues in the SLE BCI
containers, which work perfectly fine without a SLE subscription.
Thus a new flag is introduced:
--log-credentials-errors. When this flag is setor if the environment variable
CONTAINER_SUSECONNECT_LOG_CREDENTIALS_ERRisset, then
container-suseconnectwill continue printing errors as it did, butthe default is now that errors with obtaining the credentials will be silently
swallowed so that users of SLE BCI don't get confused by this error message.
This fixes #62