Skip to content

Return handler and job_runner_name to admins from the job show endpoint - #23456

Merged
jmchilton merged 1 commit into
galaxyproject:devfrom
afgane:expose-job-runner-name-to-admins
Sep 4, 2026
Merged

Return handler and job_runner_name to admins from the job show endpoint#23456
jmchilton merged 1 commit into
galaxyproject:devfrom
afgane:expose-job-runner-name-to-admins

Conversation

@afgane

@afgane afgane commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

JobSummary documents handler and job_runner_name as "Only
administrator can see this value", but GET /api/jobs/{id} never returns
them — an admin always gets null for both, while external_id and
command_line on the same response are populated.

Cause

Job.to_dict gates the two groups differently:

if system_details or view == "admin_job_list":
    rval["external_id"] = self.job_runner_external_id
    rval["command_line"] = self.command_line
    rval["traceback"] = self.traceback
if view == "admin_job_list":
    rval["handler"] = self.handler
    rval["job_runner_name"] = self.job_runner_name

GET /api/jobs/{id} goes through view_show_job, which calls
to_dict("element", system_details=is_admin). That reaches the first
branch but never the second, so the two fields are simply absent from the
dict. Because the pydantic models declare them Optional[str] = None,
they serialize as null rather than being omitted, which makes it look
like the values are missing rather than never populated.

Fix

Move handler and job_runner_name into the system_details branch.
The admin_job_list view is unaffected — its condition already includes
system_details, so it keeps returning both.

Why this matters

Anything that reads jobs one at a time loses the runner. The AnVIL tool
test harness records the full job object for every test it runs, so every
report it publishes carries a job_runner_name column that is null on
every row, and which runner a failing job used cannot be recovered after
the instance is gone.

Testing

Adds test_show_system_details_admin_only, asserting both fields are
None for a non-admin and non-None for an admin on the show endpoint.
It uses __history_with_new_dataset, which waits for job completion, so
the values are reliably set by then.

🤖 Generated with Claude Code

JobSummary documents both fields as "Only administrator can see this
value", but Job.to_dict only populated them for the admin_job_list view.
GET /api/jobs/{id} goes through view_show_job, which uses
to_dict("element", system_details=is_admin), so an admin reading a
single job got external_id, command_line and traceback but always null
for handler and job_runner_name.

Move both into the system_details branch. The admin_job_list view is
unaffected: its condition already includes system_details.

This matters for anything that reads jobs one at a time. The AnVIL tool
test harness records the full job object for every test it runs, so its
reports carry a job_runner_name column that is null on every row, and
which job runner a failing job used cannot be recovered afterwards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jmchilton jmchilton 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.

If the API tests are fine, this looks great to me.

@github-actions github-actions Bot added area/testing area/database Galaxy's database or data access layer area/testing/api labels Sep 4, 2026
@github-actions github-actions Bot added this to the 26.2 milestone Sep 4, 2026
@jmchilton
jmchilton merged commit ea5a696 into galaxyproject:dev Sep 4, 2026
56 of 59 checks passed
@github-project-automation github-project-automation Bot moved this from Needs Review to Done in Galaxy Dev - weeklies Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

This PR was merged without a "kind/" label, please correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Development

Successfully merging this pull request may close these issues.

3 participants