Skip to content

Show queue length in show-*-queue targets - #722

Merged
lbarcziova merged 1 commit into
packit:mainfrom
lbarcziova:show-queue-length
Jul 31, 2026
Merged

Show queue length in show-*-queue targets#722
lbarcziova merged 1 commit into
packit:mainfrom
lbarcziova:show-queue-length

Conversation

@lbarcziova

Copy link
Copy Markdown
Member

No description provided.

@lbarcziova
lbarcziova requested a review from majamassarini July 29, 2026 12:48
@qodo-for-packit

Copy link
Copy Markdown

PR Summary by Qodo

Show queue length in show-*-queue Makefile targets

✨ Enhancement 🕐 Less than 5 minutes

Grey Divider

AI Description

• Adds a queue length line (via valkey-cli LLEN) to the show_queue and show_single_queue
 Makefile macros.
• Prints the length for both the priority (_todo) and normal queue variants.
• Provides quick visibility into queue size without a separate manual LLEN command.
High-Level Assessment

The change is a minimal, consistent addition using the existing valkey-cli/oc exec pattern already used to list queue contents; no alternative approach is warranted for this trivial diagnostic addition.

Files changed (1) +3 / -0

Enhancement (1) +3 / -0
MakefilePrint queue length alongside contents in show-queue targets +3/-0

Print queue length alongside contents in show-queue targets

• Adds an 'echo "length: ..."' line using 'oc exec deployment/valkey -- valkey-cli --raw LLEN' for each queue dumped by the show_queue and show_single_queue macros, covering the priority todo queue, the normal queue, and single queues.

openshift/Makefile

@qodo-for-packit

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 7 rules

Grey Divider


Remediation recommended

1. LLEN failure masked 🐞 Bug ☼ Reliability
Description
The new length lines run oc exec ... LLEN inside echo $$(...), so if oc exec/valkey-cli
fails the recipe still exits successfully and can print an empty/partial length. This undermines the
Makefile’s intent to surface oc exec failures and can mislead operators when the length query
fails but the target appears to succeed.
Code

openshift/Makefile[16]

+@echo "length: $$(oc exec deployment/valkey -- valkey-cli --raw LLEN $(1)_todo)"
Relevance

●●● Strong

Team previously accepted hardening to surface oc exec failures (PR #584); likely to accept
unmasking LLEN errors too.

PR-#584
PR-#698

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The Makefile is configured to use bash with pipefail specifically to avoid masking oc exec
failures in pipelines, but the newly added echo "length: $$(oc exec ... LLEN ...)" lines use
command substitution rather than a pipeline/checked command, so failures in the substitution can be
hidden while echo still succeeds.

openshift/Makefile[1-6]
openshift/Makefile[13-21]
openshift/Makefile[23-28]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The queue length is printed via `echo "length: $$(oc exec ... LLEN ...)"`. In bash, `echo` typically returns success even if the command substitution fails, so `oc exec`/`valkey-cli` errors can be masked and the make target can appear successful while printing an empty/incorrect length.

### Issue Context
This Makefile explicitly sets bash + `pipefail` to avoid masking failures in `oc ... | jq | tac` pipelines, but the new `LLEN` calls are not pipelines and therefore don’t benefit from that protection.

### Fix Focus Areas
- openshift/Makefile[13-28]

### Suggested change
Replace the command substitution with a form that returns the `oc exec` exit status, e.g.:

- `@printf 'length: '; oc exec deployment/valkey -- valkey-cli --raw LLEN $(1)_todo`
- `@printf 'length: '; oc exec deployment/valkey -- valkey-cli --raw LLEN $(1)`
- `@printf 'length: '; oc exec deployment/valkey -- valkey-cli --raw LLEN $(1)`

(or `@echo -n 'length: '; oc exec ...`), applied consistently to all three added length lines.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread openshift/Makefile
define show_queue
@echo "=== $(1)_todo (ymir_todo, priority — popped first) ==="
@oc exec deployment/valkey -- valkey-cli --raw LRANGE $(1)_todo 0 -1 | jq -r '(.metadata.jira_issue // .metadata.issue // empty) | if type=="object" then (.key // empty) else . end' | $(TAC)
@echo "length: $$(oc exec deployment/valkey -- valkey-cli --raw LLEN $(1)_todo)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Llen failure masked 🐞 Bug ☼ Reliability

The new length lines run oc exec ... LLEN inside echo $$(...), so if oc exec/valkey-cli
fails the recipe still exits successfully and can print an empty/partial length. This undermines the
Makefile’s intent to surface oc exec failures and can mislead operators when the length query
fails but the target appears to succeed.
Agent Prompt
### Issue description
The queue length is printed via `echo "length: $$(oc exec ... LLEN ...)"`. In bash, `echo` typically returns success even if the command substitution fails, so `oc exec`/`valkey-cli` errors can be masked and the make target can appear successful while printing an empty/incorrect length.

### Issue Context
This Makefile explicitly sets bash + `pipefail` to avoid masking failures in `oc ... | jq | tac` pipelines, but the new `LLEN` calls are not pipelines and therefore don’t benefit from that protection.

### Fix Focus Areas
- openshift/Makefile[13-28]

### Suggested change
Replace the command substitution with a form that returns the `oc exec` exit status, e.g.:

- `@printf 'length: '; oc exec deployment/valkey -- valkey-cli --raw LLEN $(1)_todo`
- `@printf 'length: '; oc exec deployment/valkey -- valkey-cli --raw LLEN $(1)`
- `@printf 'length: '; oc exec deployment/valkey -- valkey-cli --raw LLEN $(1)`

(or `@echo -n 'length: '; oc exec ...`), applied consistently to all three added length lines.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@majamassarini majamassarini left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I think we can live even without length output.

Assisted-by: Claude Opus 4.6
@lbarcziova
lbarcziova merged commit df9014f into packit:main Jul 31, 2026
26 of 29 checks passed
@lbarcziova
lbarcziova deleted the show-queue-length branch July 31, 2026 08:47
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.

2 participants