-
Notifications
You must be signed in to change notification settings - Fork 993
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
Fixes #36938 - Ignore output of some commands in Global Registration #9914
Conversation
b999a8b
to
7e97f27
Compare
@nofaralfasi review please |
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.
LGTM
@@ -175,7 +175,7 @@ chmod 644 $KATELLO_SERVER_CA_CERT | |||
if ! [ -x "$(command -v subscription-manager)" ] ; then | |||
$PKG_MANAGER_INSTALL subscription-manager |
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.
Don't you need to silence this command too?
There are also subscription-manager config
commands later in this script, do you know if they need to be silenced too?
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 reproduced the reported issue and tested it with the changes in this PR. For me, the current modifications cleared all the unnecessary messages.
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.
$PKG_MANAGER_INSTALL subscription-manager
this one no, because if installation of subman fails, users need to know about it.
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.
OK, on a second thought if sub-man is not installed on the system, package manager should not complain about subscriptions, since the plugin that does that, would not be installed at all.
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.
Didn't test it, but LGTM
No description provided.