MGMT-20958: Logging and error handling#12
Merged
carbonin merged 6 commits intoopenshift-assisted:masterfrom Jul 7, 2025
Merged
MGMT-20958: Logging and error handling#12carbonin merged 6 commits intoopenshift-assisted:masterfrom
carbonin merged 6 commits intoopenshift-assisted:masterfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@carbonin: This pull request references MGMT-20958 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: carbonin The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
""" WalkthroughThe changes introduce comprehensive logging and error handling improvements across the codebase. Logging is now more detailed in both API client and server modules, with structured log messages and robust exception handling. The logging system is also updated to support environment-based configuration, allowing file logging to be toggled via an environment variable, which is now set in the Dockerfile. Additionally, the Makefile limits container port exposure to localhost. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Server
participant InventoryClient
participant AssistedServiceAPI
User->>Server: Send API request
Server->>Server: Log request start
Server->>InventoryClient: Call API method
InventoryClient->>InventoryClient: Log API call start
InventoryClient->>AssistedServiceAPI: Perform API request
AssistedServiceAPI-->>InventoryClient: Respond (or error)
InventoryClient->>InventoryClient: Log success or error
InventoryClient-->>Server: Return result or raise exception
Server->>Server: Log operation completion or error
Server-->>User: Return response
Suggested labels
Poem
""" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
openshift-merge-robot
added
the
needs-rebase
openshift-ci
Bot
added
the
size/XL
carbonin
force-pushed
the
logging-and-error-handling
branch
from
ad06674 to
5ddc039
Compare
openshift-merge-robot
removed
the
needs-rebase
|
@carbonin: This pull request references MGMT-20958 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
carbonin
force-pushed
the
logging-and-error-handling
branch
from
6605e63 to
85f077b
Compare
|
@carbonin: This pull request references MGMT-20958 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Without this I was unable to access the service using an MCP client or curl
|
@carbonin: This pull request references MGMT-20958 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Makefile (1)
14-14: Parameterize host binding for greater flexibilityHard-coding
127.0.0.1improves security, but it also limits scenarios where developers may legitimately want external access (e.g., running demos on a LAN). Expose the host binding as a Make variable so callers can override it without editing the file.+HOST_BIND ?= 127.0.0.1 @@ - podman run --rm -p 127.0.0.1:8000:8000 $(IMAGE_NAME):$(TAG) + podman run --rm -p $(HOST_BIND):8000:8000 $(IMAGE_NAME):$(TAG)This keeps the default secure while remaining configurable via
make run HOST_BIND=0.0.0.0.
eranco74
reviewed
Jul 6, 2025
| log.debug("Found offline token in request headers") | ||
| return token | ||
|
|
||
| log.error("No offline token found in environment or request headers") |
Collaborator
There was a problem hiding this comment.
Unrelated to this PR, not having OCM offline token set shouldn't lead to RuntimeError.
Something seems off here.
Collaborator
|
/lgtm |
Merged
Open
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
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.
Add better logging for when tools are run.
Additionally add better error handling when calling the assisted service APIs.
Finally standardize the log formatting and remove file logging when running in a container.
After this change logging looks like:
Summary by CodeRabbit
New Features
Chores