Add worker info in job_next_previous

[ggardet]

This PR adds a Worker column to know which worker was used.

Screenshot:

[Martchus]

It would make more sense to show the worker's host name and instance number as link text because the ID is rather random. You could also already make the href server-side using the url_for helper of Mojolicious.

I'm also wondering whether the JavaScript code that generates HTML should take care of escaping. We're not doing it for other columns so far but it would be an easy improvement as we generally already have a helper function for this.

[ggardet]

It would make more sense to show the worker's host name and instance number as link text because the ID is rather random.

That was my original idea, but I still need to figure out how to retrieve the name.

You could also already make the href server-side using the url_for helper of Mojolicious.

I will have a look.

[ggardet]

PR updated to use worker name instead of worker id, based on @kalikiana suggested change. (Thanks!)

[Martchus]

Have you tested the performance impact? This will do yet another SQL query for every job for this already slow route. The "prefetching" I meant in another comment is basically a join with the workers table when querying jobs. I suppose the "prefetching" needed to be done in

openQA/lib/OpenQA/Schema/ResultSet/Jobs.pm

Line 399 in 9cf9ed8

sub next_previous_jobs_query {

using some DBIx magic. Maybe this doesn't work in this special case, though. Then you'd likely need to touch the raw SQL code in https://github.com/os-autoinst/openQA/blob/9cf9ed8fc9feb80490887185df8d0af2c0c5b603/lib/OpenQA/Schema/Result/JobNextPrevious.pm.

[ggardet]

Not sure how I could measure the perf impact.

The SQL code has already SELECT *, so I think it should be pre-fetched already, no?

[Martchus]

No, it needed a join or a nested query with the workers table to return the associated worker name. However, I'd needed to play around with this myself because using a view is a special case.

For performance testing itself, I usually import a production database locally. This isn't an option for your but you could simply create lots of jobs (that are next/previous ones for an existing job) manually. To measure the response time of web UI routes I usually use a Mojolicious plugin (see https://github.com/Martchus/openQA-helper#profiling).

Note that I'm only concerned about this because this route already takes long anyways (e.g. checkout https://openqa.opensuse.org/tests/3126056#next_previous) and if we'd multiply this time even by a slight factor due to an additional/separate query per job this feature might do more harm than good.

[okurz]

Considering the performance impact from #4990 (comment) as well as the user experience I think we are able to come up with a better approach. I understand that sometimes someone would want to know the worker info from next+previous jobs but likely the purpose is "investigation" and we have a separate page for that. I guess in other cases people want to know other, different page in the next+previous page. But putting all of that information in the page from the beginning can be too much. Alternative approaches I suggest to consider:

1. Include the information in the "investigation" page instead
2. Add a table header or check box with what additional information items to include on request but keeping the page with the information as is as the default

[Martchus]

Another alternative: We could include at least a link to the worker's page as the worker ID is available (without doing any further queries or joins). Since the link wouldn't have a meaningful text (just the worker ID) it could be in form of an icon.

[okurz]

That alternative suggestion sounds good to me. A good compromise UX- and performance wise

[ggardet]

It does not make much sense to have it in investigation tab since you cannot reach it when the test is green.

I need to investigate how to add a checkbox for this optional column.

[Martchus]

Having an optional column might not be that easy (depending how well DataTables supports that). So maybe just go for an icon-link as suggested in my previous comment.

[ggardet]

Having an optional column might not be that easy (depending how well DataTables supports that). So maybe just go for an icon-link as suggested in my previous comment.

Worker ID was my initial submission but this is far less useful than worker name.
Maybe we can build a table to translate worker ID to worker name.

[Martchus]

but this is far less useful than worker name.

If it is a link to the worker details page of that specific worker it is still an improvement.

Maybe we can build a table to translate worker ID to worker name.

We have that table, it is the workers table. And accessing the worker via …->assigned_worker->.… will automatically do a query to that table. The problem is just that doing this query for every job will likely have a notable impact on the response time (and so far you haven't shown any figures which suggest otherwise). I've already proposed to do a join (or sub query) in the initial query for jobs. It would still be slower but likely the database will be able to handle it efficient enough internally and we'd still only do one query. However, doing that definitely requires some playing around with DBIx and the raw SQL we use.

[perlpunk]

#5043 optimizes the existing next/previous route with prefetching job modules and dependencies.
We could try out the same with workers and see how it performs.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.