Skip to content

Comments

Add status command to verify subscription and daemon operations#348

Draft
r0x0d wants to merge 2 commits intorhel-lightspeed:mainfrom
r0x0d:status-command
Draft

Add status command to verify subscription and daemon operations#348
r0x0d wants to merge 2 commits intorhel-lightspeed:mainfrom
r0x0d:status-command

Conversation

@r0x0d
Copy link
Member

@r0x0d r0x0d commented Apr 25, 2025

The status command will be helpful to aid users to identify if their system is operation correctly. The current command is able to verify subscription status, a few daemon operations and will be able to do more in the future.

Closes: #343

Checklist

  • Jira issue has been made public if possible
  • [RSPEED-] is part of the PR title
  • PR title explains the change from the user's point of view
  • Code and tests are documented properly
  • The commits are squashed to as few commits as possible (without losing data)

@r0x0d r0x0d requested a review from a team as a code owner April 25, 2025 13:53
@codecov
Copy link

codecov bot commented Apr 25, 2025

Codecov Report

Attention: Patch coverage is 48.92086% with 71 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
command_line_assistant/commands/status.py 42.97% 69 Missing ⚠️
command_line_assistant/dbus/interfaces/base.py 71.42% 2 Missing ⚠️
Files with missing lines Coverage Δ
command_line_assistant/dbus/interfaces/chat.py 100.00% <100.00%> (ø)
command_line_assistant/dbus/interfaces/history.py 96.92% <100.00%> (+0.04%) ⬆️
command_line_assistant/dbus/interfaces/user.py 100.00% <100.00%> (ø)
command_line_assistant/exceptions.py 100.00% <100.00%> (ø)
command_line_assistant/initialize.py 93.47% <100.00%> (+0.14%) ⬆️
command_line_assistant/utils/cli.py 97.26% <100.00%> (+0.03%) ⬆️
command_line_assistant/dbus/interfaces/base.py 71.42% <71.42%> (ø)
command_line_assistant/commands/status.py 42.97% <42.97%> (ø)

)
)

def _check_subscription_status(self) -> None:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Another application staring to use our D-Bus API.

The status command will be helpful to aid users to identify if their
system is operation correctly. The current command is able to verify
subscription status, a few daemon operations and will be able to do more
in the future.
@subpop
Copy link
Member

subpop commented Apr 28, 2025

I have another suggestion. Instead of adding more dependencies on the com.redhat.RHSM1 D-Bus interfaces and binding together more applications through D-Bus APIs, could we leave the RHSM/subscription status check to the RHSM/subscription status client, and make the cla status command just test connectivity to the CLA API? It's redundant to have c status check the status of RHSM/subscriptions, and it's kind of tangential to the tool's responsibility. I know that a valid subscription is required to reach the CLA API, but I feel like we should leave the actual subscription status check to RHSM's clients. If we get an HTTP 401 back from our API check, we could log a helpful error like "Unable to authorize a connection to console.redhat.com. Ensure you have an active subscription by running 'subscription-manager status'".

@r0x0d
Copy link
Member Author

r0x0d commented Apr 28, 2025

I have another suggestion. Instead of adding more dependencies on the com.redhat.RHSM1 D-Bus interfaces and binding together more applications through D-Bus APIs, could we leave the RHSM/subscription status check to the RHSM/subscription status client, and make the cla status command just test connectivity to the CLA API? It's redundant to have c status check the status of RHSM/subscriptions, and it's kind of tangential to the tool's responsibility. I know that a valid subscription is required to reach the CLA API, but I feel like we should leave the actual subscription status check to RHSM's clients. If we get an HTTP 401 back from our API check, we could log a helpful error like "Unable to authorize a connection to console.redhat.com. Ensure you have an active subscription by running 'subscription-manager status'".

That's a really good point. We can totally do that.

I just started this out last week and was hacking a couple of things together. Should have made the PR a draft.

@r0x0d r0x0d marked this pull request as draft April 28, 2025 16:30
@r0x0d
Copy link
Member Author

r0x0d commented Apr 28, 2025

TODO

  • Check with John Carr about the UI/UX for this command
  • Use @subpop suggestion to just make a request and verify if the certificates are correct
  • Verify permissions of the certificate (probably want to create a new interface in dbus)

@jirihnidek
Copy link

I have another suggestion. Instead of adding more dependencies on the com.redhat.RHSM1 D-Bus interfaces and binding together more applications through D-Bus APIs, could we leave the RHSM/subscription status check to the RHSM/subscription status client, and make the cla status command just test connectivity to the CLA API? It's redundant to have c status check the status of RHSM/subscriptions, and it's kind of tangential to the tool's responsibility. I know that a valid subscription is required to reach the CLA API, but I feel like we should leave the actual subscription status check to RHSM's clients. If we get an HTTP 401 back from our API check, we could log a helpful error like "Unable to authorize a connection to console.redhat.com. Ensure you have an active subscription by running 'subscription-manager status'".

I would rather recommend to use rhc status --format json. It is easier to parse and the rhc is more preferred CLI tool on RHEL.

@subpop
Copy link
Member

subpop commented Apr 29, 2025

I would rather recommend to use rhc status --format json. It is easier to parse and the rhc is more preferred CLI tool on RHEL.

Does rhc in RHEL 9.6 support the status --format command? I can't remember when we added it and how far back we back-ported it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add status command

3 participants