Skip to content

Improve type annotation of job runners#21050

Merged
mvdbeek merged 1 commit intogalaxyproject:devfrom
nsoranzo:job_runners_type_annot
Oct 14, 2025
Merged

Improve type annotation of job runners#21050
mvdbeek merged 1 commit intogalaxyproject:devfrom
nsoranzo:job_runners_type_annot

Conversation

@nsoranzo
Copy link
Copy Markdown
Member

Including various related fixes.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@nsoranzo nsoranzo requested a review from Copilot October 14, 2025 12:15
@github-actions github-actions Bot added this to the 26.0 milestone Oct 14, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request improves type annotations of job runners by adding proper type hints and updating various job runner classes for better type safety. The changes also include several related bug fixes and code improvements.

Key changes:

  • Added proper type annotations to job runner constructors, methods, and parameters
  • Fixed type-related issues in test files by adding proper type casting
  • Updated AsynchronousJobState parameter ordering and improved type safety

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/unit/app/jobs/test_runner_local.py Added type casting for MinimalJobWrapper compatibility
test/unit/app/jobs/test_job_wrapper.py Fixed type annotations and removed type ignore comments
lib/galaxy/jobs/runners/univa.py Updated type annotations for DRMAAJobState
lib/galaxy/jobs/runners/tasks.py Added comprehensive type annotations and assertions
lib/galaxy/jobs/runners/slurm.py Fixed return type annotations and added explicit returns
lib/galaxy/jobs/runners/pulsar.py Improved type safety and fixed parameter ordering
lib/galaxy/jobs/runners/pbs.py Updated type annotations and parameter consistency
lib/galaxy/jobs/runners/local.py Enhanced type annotations and added type ignore comments
lib/galaxy/jobs/runners/kubernetes.py Fixed parameter ordering and improved type annotations
lib/galaxy/jobs/runners/godocker.py Updated type annotations and fixed parameter ordering
lib/galaxy/jobs/runners/drmaa.py Added DRMAAJobState class and improved type safety
lib/galaxy/jobs/runners/condor.py Enhanced constructor type annotations and fixed typo
lib/galaxy/jobs/runners/cli.py Added type annotations and updated parameter ordering
lib/galaxy/jobs/runners/chronos.py Fixed type annotations and parameter ordering
lib/galaxy/jobs/runners/aws.py Updated type annotations and parameter ordering
lib/galaxy/jobs/runners/init.py Added generic typing and improved base class annotations
lib/galaxy/jobs/init.py Enhanced type annotations for job wrapper classes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread lib/galaxy/jobs/runners/pbs.py
_write_logfile(job_state.error_file, reason)
job_state.running = False
job_state.stop_job = True
job_state.stop_job = False
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

The assignment should be job_state.stop_job = True to properly stop the job, not False.

Suggested change
job_state.stop_job = False
job_state.stop_job = True

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, this signals to the stop_job() method that the job on the cluster has already terminated and doesn't need to be stopped.

@mvdbeek mvdbeek merged commit a55f2a1 into galaxyproject:dev Oct 14, 2025
57 of 61 checks passed
@nsoranzo nsoranzo deleted the job_runners_type_annot branch October 14, 2025 15:48
@galaxyproject-sentryintegration
Copy link
Copy Markdown

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

Did you find this useful? React with a 👍 or 👎

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants