Add all job states to be monitored from /tests route #1446

foursixnine · 2017-09-07T09:19:02Z

This is an approach of adding all the states to the /tests route, so that wen can see all tests that can be represented through any of OpenQA::Schema::Result::Jobs::STATES.

While it seems to be feasible, there are few things that migh need to change (Including how we are representing the tests at all). Also not to do unnecesary queries thatspam the database every time someone is opening /tests.

foursixnine · 2017-09-07T09:40:15Z

lib/OpenQA/WebAPI/Controller/Test.pm

-    @scheduled = sort { $b->t_created <=> $a->t_created || $b->id <=> $a->id } @scheduled;
+    # @scheduled = sort { $b->t_created || 'Inf' <=> $a->t_created || 'Inf' || $b->id <=> $a->id } @scheduled;
+    @scheduled = sort {
+        if (defined($b->{job}->t_created) && defined($a->{job}->t_created)) {


I believe that with the last change of moving ASSIGNED to PRE_EXECUTION_STATES fixed the problems I was having, rendering this change useless :) (And same with the other sorts).

okurz · 2017-09-17T20:00:00Z

lib/OpenQA/WebAPI/Controller/Test.pm

+        else {
+            0;
+        }
+    } @scheduled;


Please DRY. You are calling the same code multiple times, please extract method.

It is actually not the same code. I guess it could be extracted so that we end up with something like $b->$time_key <=> $a->$time_key || $b->id <=> $a->id

okurz · 2017-09-17T20:00:38Z

templates/test/running_table.html.ep

@@ -35,7 +35,7 @@
            % my $distri = $job->DISTRI // '';
            % my $version = $job->VERSION // '';
            % my $flavor = $job->FLAVOR // '';
-	    % my $arch = $job->ARCH // '';
+        % my $arch = $job->ARCH // '';


something seems wrong here

okurz · 2017-09-17T20:00:58Z

templates/test/running_table.html.ep

@@ -48,12 +48,12 @@
                    of <%= "$distri-$version-$flavor.$arch" %>
                </td>
                <td class="test">
-		    % if (is_operator) {
-			%= link_to url_for('apiv1_cancel', jobid => $job->id) => (class => 'cancel') => begin
+            % if (is_operator) {


so why these indendation change?

that's tabs I assume?

I see.

@foursixnine Please update the git commit message subject line of "Changes in formatting of the template" to something more sexy. And I guess you need to ensure indendation is correct after the tab replacement

So I'll fix that commit message, with the commit description then Replace tabs with spaces and fix indentation fixes the operator indentation too

codecov · 2017-09-20T12:40:28Z

Codecov Report

Merging #1446 into master will decrease coverage by 0.03%.
The diff coverage is 62.5%.

@@            Coverage Diff             @@
##           master    #1446      +/-   ##
==========================================
- Coverage   88.47%   88.44%   -0.04%     
==========================================
  Files         105      105              
  Lines        7945     7950       +5     
==========================================
+ Hits         7029     7031       +2     
- Misses        916      919       +3

Impacted Files	Coverage Δ
lib/OpenQA/Schema/Result/Jobs.pm	`90.69% <100%> (+0.01%)`	⬆️
lib/OpenQA/WebAPI/Controller/Test.pm	`98.28% <40%> (-1.03%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 696bb63...546f998. Read the comment docs.

codecov · 2017-09-20T12:40:28Z

Codecov Report

Merging #1446 into master will increase coverage by 13.2%.
The diff coverage is 62.5%.

@@            Coverage Diff             @@
##           master    #1446      +/-   ##
==========================================
+ Coverage   75.27%   88.47%   +13.2%     
==========================================
  Files         105      105              
  Lines        7936     7941       +5     
==========================================
+ Hits         5974     7026    +1052     
+ Misses       1962      915    -1047

Impacted Files	Coverage Δ
lib/OpenQA/Schema/Result/Jobs.pm	`90.69% <100%> (+11.87%)`	⬆️
lib/OpenQA/WebAPI/Controller/Test.pm	`98.28% <40%> (-1.03%)`	⬇️
lib/OpenQA/WebAPI/Plugin/Helpers.pm	`97.14% <0%> (+0.57%)`	⬆️
lib/OpenQA/WebAPI.pm	`92.18% <0%> (+2.28%)`	⬆️
lib/OpenQA/Schema/Result/ScreenshotLinks.pm	`39.62% <0%> (+5.66%)`	⬆️
lib/OpenQA/Schema/Result/Needles.pm	`79.04% <0%> (+5.71%)`	⬆️
lib/OpenQA/WebAPI/Controller/API/V1/Job.pm	`79.31% <0%> (+6.03%)`	⬆️
lib/OpenQA/Utils.pm	`90.65% <0%> (+7.16%)`	⬆️
lib/OpenQA/Schema/Result/JobModules.pm	`91.16% <0%> (+7.9%)`	⬆️
lib/OpenQA/Schema/Result/Workers.pm	`96.25% <0%> (+8.75%)`	⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a0fcfc...805ec4a. Read the comment docs.

foursixnine · 2017-09-22T08:31:46Z

I have just updated the PR description with an image of how it will look like.

In the very seldom case where a job has t_started = undef, a warning will show up. But other than that I see no more problems

okurz

I recommend to revert the indendation changes, rest +1

okurz · 2017-09-22T08:51:18Z

templates/test/running_table.html.ep

@@ -48,12 +48,12 @@
                    of <%= "$distri-$version-$flavor.$arch" %>
                </td>
                <td class="test">
-		    % if (is_operator) {
-			%= link_to url_for('apiv1_cancel', jobid => $job->id) => (class => 'cancel') => begin
+            % if (is_operator) {


so why these indendation change?

With all the STATES we have (and those forthcomming), we need to know better what states belong to whic slice of it's lifetime. Remove hardcoded states Move Assigned back to EXECUTION Right now, a job that is assigned is a job that is "pending" execution For the time being we still have some jobs that spend some time here, this will allow to easily restart as 4eb1114 did.

Code was using .targets as a positional argument (Column name instead of a CSS class), this causes the table to change if a new column is being added. https://datatables.net/reference/option/columnDefs.targets

commit 92f4fa1 Merge: a8e5e97 546f998 Author: Santiago Zarate <santiago@zarate.net.ve> AuthorDate: Wed Sep 27 17:43:50 2017 +0200 Commit: GitHub <noreply@github.com> CommitDate: Wed Sep 27 17:43:50 2017 +0200 Merge pull request #1446 from foursixnine/feature/job_table_revamp Add all job states to be monitored from /tests route

AdamWill · 2017-10-10T14:45:30Z

Once again, a plea can you please @ me when changing the job state (etc.) constants? They need to be updated in the Python client also. Thanks!

coolo · 2017-10-11T04:39:52Z

this PR doesn't though - it just shuffles around the internal arrays

AdamWill · 2017-10-11T17:19:30Z

oh right, I misread the changes. still, point stands - something changed 'em recently and I had to spot it myself :P

foursixnine changed the title Add Status field to the scheduled table Changes in formatting of the template Proposal to change the sort of scheduled and running jobs Replace tabs with spaces Remove hardcoded states Better reflect job execution through states Revamping of /tests Sep 7, 2017

foursixnine changed the title ~~Revamping of /tests~~ WIP: Revamping of /tests Sep 7, 2017

foursixnine commented Sep 7, 2017

View reviewed changes

mudler mentioned this pull request Sep 7, 2017

Treat ASSIGNED state as in execution #1447

Merged

foursixnine force-pushed the feature/job_table_revamp branch 5 times, most recently from 7f644a1 to 3316069 Compare September 13, 2017 14:36

okurz reviewed Sep 17, 2017

View reviewed changes

foursixnine force-pushed the feature/job_table_revamp branch 3 times, most recently from e93b16e to 805ec4a Compare September 19, 2017 14:45

foursixnine changed the title ~~WIP: Revamping of /tests~~ Add all job states to be monitored from /tests route Sep 21, 2017

foursixnine force-pushed the feature/job_table_revamp branch from 805ec4a to 7e12cfc Compare September 22, 2017 08:26

okurz reviewed Sep 22, 2017

View reviewed changes

foursixnine added 4 commits September 22, 2017 11:35

Replace tabs with spaces and fix indentation.

7eebd17

Ensure CSS classes are used for Datatables

0e83236

Code was using .targets as a positional argument (Column name instead of a CSS class), this causes the table to change if a new column is being added. https://datatables.net/reference/option/columnDefs.targets

Add setup state to be polled and tidy up

546f998

foursixnine force-pushed the feature/job_table_revamp branch from 7e12cfc to 546f998 Compare September 22, 2017 09:40

mudler approved these changes Sep 27, 2017

View reviewed changes

foursixnine merged commit 92f4fa1 into os-autoinst:master Sep 27, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add all job states to be monitored from /tests route #1446

Add all job states to be monitored from /tests route #1446

foursixnine commented Sep 7, 2017 •

edited

foursixnine Sep 7, 2017

okurz Sep 17, 2017

foursixnine Sep 19, 2017

okurz Sep 17, 2017

okurz Sep 17, 2017

okurz Sep 22, 2017

coolo Sep 22, 2017

okurz Sep 22, 2017

foursixnine Sep 22, 2017

codecov bot commented Sep 20, 2017 •

edited

codecov bot commented Sep 20, 2017

foursixnine commented Sep 22, 2017

okurz left a comment

okurz Sep 22, 2017

AdamWill commented Oct 10, 2017

coolo commented Oct 11, 2017

AdamWill commented Oct 11, 2017

Add all job states to be monitored from /tests route #1446

Add all job states to be monitored from /tests route #1446

Conversation

foursixnine commented Sep 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 20, 2017 • edited

Codecov Report

codecov bot commented Sep 20, 2017

Codecov Report

foursixnine commented Sep 22, 2017

okurz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdamWill commented Oct 10, 2017

coolo commented Oct 11, 2017

AdamWill commented Oct 11, 2017

foursixnine commented Sep 7, 2017 •

edited

codecov bot commented Sep 20, 2017 •

edited