Skip to content

fix: enable PositionalArgumentsFormatter in logger#1307

Merged
jgarciao merged 1 commit intoopendatahub-io:mainfrom
jgarciao:fix-logging-with-possitional-args
Mar 26, 2026
Merged

fix: enable PositionalArgumentsFormatter in logger#1307
jgarciao merged 1 commit intoopendatahub-io:mainfrom
jgarciao:fix-logging-with-possitional-args

Conversation

@jgarciao
Copy link
Copy Markdown
Contributor

@jgarciao jgarciao commented Mar 26, 2026

Fixes positional arguments not being formatted properly in the logs

LOGGER.info(
        "Uploading dataset (%d document(s)) to vector_store (id=%s)",
        len(dataset.documents),
        vector_store.id,
    )

Summary by CodeRabbit

  • Chores
    • Enhanced logging infrastructure to improve how arguments are formatted and displayed in log messages.

Fixes possitional arguments not being formatted properly in the
logs

LOGGER.info(
        "Uploading dataset (%d document(s)) to vector_store (id=%s)",
        len(dataset.documents),
        vector_store.id,
    )

Signed-off-by: Jorge Garcia Oncins <jgarciao@redhat.com>
@jgarciao jgarciao requested a review from a team as a code owner March 26, 2026 11:26
@github-actions
Copy link
Copy Markdown

The following are automatically added/executed:

  • PR size label.
  • Run pre-commit
  • Run tox
  • Add PR author as the PR assignee
  • Build image based on the PR

Available user actions:

  • To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
  • To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
  • To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
    lgtm label removed on each new commit push.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
    and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
  • To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
    pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.
Supported labels

{'/build-push-pr-image', '/verified', '/lgtm', '/hold', '/wip', '/cherry-pick'}

@fege
Copy link
Copy Markdown
Contributor

fege commented Mar 26, 2026

/lgtm

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: a724fce8-54be-4843-a218-abd697183087

📥 Commits

Reviewing files that changed from the base of the PR and between 8a67a44 and 1ae06ca.

📒 Files selected for processing (1)
  • utilities/logger.py

📝 Walkthrough

Walkthrough

Added structlog.stdlib.PositionalArgumentsFormatter() processor to the shared structlog pipeline in utilities/logger.py. This formatter processes positional arguments and merges them into log event data before downstream processors execute.

Changes

Cohort / File(s) Summary
Structlog Pipeline Configuration
utilities/logger.py
Added PositionalArgumentsFormatter() to _SHARED_PROCESSORS pipeline to format positional arguments before timestamping and stdlib integration.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Security considerations: Verify that positional arguments passed to logging calls don't contain untrusted data that could expose sensitive information. If positional args originate from external input, ensure they're sanitized prior to logging to prevent information disclosure or injection attacks.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description lacks required sections from the template including Related Issues, testing checkboxes, and Additional Requirements sections. Add missing template sections: link Related Issues (Fixes/JIRA), include testing checkboxes (Locally/Jenkins), and Additional Requirements checkboxes.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: enabling PositionalArgumentsFormatter in the logger to fix how positional arguments are formatted.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jgarciao jgarciao requested review from a team and Bobbins228 March 26, 2026 11:37
@jgarciao jgarciao merged commit 538e73a into opendatahub-io:main Mar 26, 2026
11 checks passed
@github-actions
Copy link
Copy Markdown

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants