Skip to content

fix: include job_id in all failure paths of fit_predict_async#128

Open
Saloni-0465 wants to merge 1 commit into
sktime:mainfrom
Saloni-0465:fix/async-job-id-missing-on-failure
Open

fix: include job_id in all failure paths of fit_predict_async#128
Saloni-0465 wants to merge 1 commit into
sktime:mainfrom
Saloni-0465:fix/async-job-id-missing-on-failure

Conversation

@Saloni-0465
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

None.


What does this implement/fix?

fit_predict_async now always includes job_id in the returned response across all paths:

  • dataset load failure
  • fit failure
  • predict failure
  • success

Previously, some early failure paths returned only an error dict without job_id, even though the job had already been created. This made it impossible for clients to track or inspect failed async jobs.

This change ensures consistency in the async job lifecycle - every response now includes job_id.


Does your contribution introduce a new dependency?

No.


What should a reviewer concentrate their feedback on?

  • Return shapes from Executor.fit_predict_async
  • Consistency of job_id across all execution paths

Any other comments?

None.


PR checklist

For all contributions

  • I've added myself to the list of contributors (if applicable)
  • I've added unit tests and made sure they pass locally

Title

@direkkakkar319-ops
Copy link
Copy Markdown

direkkakkar319-ops commented Apr 11, 2026

hi @Saloni-0465 i saw that you lint and pytests are failing this is due to problem in the main branch i have fixed both problem in my recent prs

@Saloni-0465
Copy link
Copy Markdown
Contributor Author

Got it @direkkakkar319-ops - I’ll rebase main once your CI fixes are in so this PR picks them up.

@Saloni-0465 Saloni-0465 force-pushed the fix/async-job-id-missing-on-failure branch from 4851560 to da161e6 Compare May 8, 2026 12:21
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.

2 participants