-
Notifications
You must be signed in to change notification settings - Fork 37
feat(cli): add job status information to queue-status command #774
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
feat(cli): add job status information to queue-status command #774
Conversation
a83662d to
df6c2d4
Compare
rene-oromtz
left a comment
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.
Hi @gajeshbhat
Thanks again for taking the time to add enhancements to this repo.
I added few suggestions based on current behavior of this command, since we are already returning jobs that are "waiting" in both json/human readable format we can extend the functionality with your proposed --verbose to include all other statuses.
Based on the enhancement request, for the human readable output, I will probably just leave it as follows to preserve the current output:
testflinger-cli queue-status <agent>
Agents in queue: 1
Available: 1
Busy: 0
Offline: 0
Jobs waiting: 0
Jobs running: 0 # new
Jobs completed: 0 # new
The json can output can also include this two new keys (jobs_running, jobs_completed) with additional timestamp information for each job.
Please let me know what you think, if you have any question regarding my comments, don't hesitate to ask!
df6c2d4 to
27835d3
Compare
Hello @rene-oromtz, Thanks for the detailed feedback! I've replied to your comments and made some changes: Changes Made:
Please let me know if you have any questions. Cheers. |
27835d3 to
9abe829
Compare
rene-oromtz
left a comment
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.
Thanks for adding the suggestions! Overall I'm liking the current approach, I just leave couple more suggestions and one important consideration regarding on the current MongoDB structure
9abe829 to
ed80dce
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #774 +/- ##
==========================================
+ Coverage 63.70% 64.49% +0.79%
==========================================
Files 94 94
Lines 7772 7946 +174
Branches 746 764 +18
==========================================
+ Hits 4951 5125 +174
+ Misses 2680 2673 -7
- Partials 141 148 +7
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
Thank you @rene-oromtz for taking the time to look into this again. I have made the requested changes and requested re-review. Please let me know if you have any questions. |
rene-oromtz
left a comment
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.
Thanks again for the changes!
Just added final nitpicks/suggestions that I didn't catch before (sorry for that, this are the final comments I have). Functionally, the code looks good to me!
Tested locally as well and its working as expected
Add job status counts and details to the existing queue-status command: - Show job running/completed counts in default output alongside agent status - Add --verbose flag to display individual job details with timestamps - Categorize jobs as waiting/running/completed (ignore cancelled jobs) - Include new JSON keys (jobs_running, jobs_completed) with timestamp info - Maintain backward compatibility with existing agent status display This addresses issue canonical#116 by providing job status summary without changing the existing command behavior or requiring additional flags. Closes canonical#116 (Internally #CERTTF-248)
ed80dce to
48ca14c
Compare
Thanks again for taking a look at this @rene-oromtz .I have updated the changes you asked for. I just wanted to let you know that there's no need to be sorry. I am happy to always work with the maintainers to make sure my code submission meets the repository standards and does not introduce more technical debt. Please let me know if you have any further questions. Cheers. :) On a side note, I have also put up a PR for a bug that troubled me a lot in the past while running testflinger in production when agents crash due to intermittent network issues. I believe this bugfix would help many teams running TestFlinger. No rush/urgency on this. Take a look when you have free time. Cheers. |
rene-oromtz
left a comment
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.
@gajeshbhat once again, thanks for the time you put on this PR, this looks good to me!
Description
This PR enhances the existing
queue-statuscommand to display job status information alongside the current agent status, addressing the feature request in issue #116.Users need a way to get a summary of job statuses in a queue without polling individual job IDs. When running many jobs in sequence, it's difficult to get a quick overview of job progress and identify active work.
The solution extends the existing
queue-statuscommand to show both agent and job status in unified output, using a simple--verboseflag for detailed job information.Resolved issues
Fixes #116
Documentation
queue-statuscommand in CLI help exampleWeb service API changes
No API changes required. Uses existing endpoints:
/v1/queues/{queue}/agentsfor agent status information/v1/queues/{queue}/jobsfor job status informationTests
How this PR was tested:
Unit Tests (50/50 passing):
Linting & Formatting:
Manual Testing:
Test coverage includes:
Key features implemented:
--verboseflag for detailed informationExample outputs:
Default (enhanced):
Verbose (new):
This implementation addresses the core need from issue #116 while maintaining full compatibility with existing usage and following established project patterns, and also helps the project close #CERTTF-248 internally.