Skip to content

Conversation

@Enkidu93
Copy link
Collaborator

@Enkidu93 Enkidu93 commented Oct 26, 2023

This solution fetches all tasks queued in the configured queue and unions them with the tasks associated with currently monitored translation engines. This means that there are two additional API calls per ClearMLMonitorService run (one to fetch the ids of tasks on the queue and another to fetch the complete info on those tasks). I think this is inevitable because the get-all-queues call only contains information about queued jobs (and, if you dig, currently running jobs), but not jobs that may have failed or completed.


This change is Reviewable

@Enkidu93 Enkidu93 requested a review from ddaspit October 26, 2023 13:09
@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Files Coverage Δ
...chine.AspNetCore/Services/ClearMLMonitorService.cs 5.15% <0.00%> (-0.05%) ⬇️
.../SIL.Machine.AspNetCore/Services/ClearMLService.cs 32.09% <0.00%> (-1.45%) ⬇️

📢 Thoughts on this report? Let us know!.

Copy link
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Enkidu93)

@Enkidu93 Enkidu93 merged commit cde5659 into master Oct 26, 2023
@Enkidu93 Enkidu93 deleted the make_queue_depth_universal branch October 26, 2023 15:52
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.

4 participants