Make Executor.get_function_and_kwargs the overridable resolution hook.

[PaulM5406]

Hey @TkTech,

I would like to be able to override get_function_and_kwargs in custom executor to be able to dynamically inject other kwargs like it is possible with QueueJob.

Thanks !

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.31%. Comparing base (4026904) to head (acb2407).
⚠️ Report is 11 commits behind head on 24_4.

Files with missing lines	Patch %	Lines
chancy/executors/sub.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             24_4      #46      +/-   ##
==========================================
+ Coverage   71.26%   71.31%   +0.05%     
==========================================
  Files          55       55              
  Lines        2899     2894       -5     
==========================================
- Hits         2066     2064       -2     
+ Misses        833      830       -3     

Flag	Coverage Δ
unittests	71.31% <80.00%> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[TkTech]

This one was originally intentional, as significant care has to be taken when crossing process/thread/sub-interpreter barriers. But your use case is definitely valid and this is definitely safe in the async executor. I'll give the others a closer review.

[PaulM5406]

Hey @TkTech, would you have the time to have another look at this PR ? May it be part of 0.24.4 ?
If not, may you add only the change of the async executor to the 0.24.4 release ?
Do you think you could release it soon ?
Thank you !

[TkTech]

#49 will release tomorrow with this supported at least for the async executor.

Could you expand on your use case for this one? Is the kwarg you want to inject related to Chancy itself, like job or queue data, or your own?

[PaulM5406]

#49 will release tomorrow with this supported at least for the async executor.

Could you expand on your use case for this one? Is the kwarg you want to inject related to Chancy itself, like job or queue data, or your own?

kwargs I want to inject is not part of chancy. There are some other external services, like other DB connections.

[luca-montaigut]

Taking over this PR from @PaulM5406, context in #58 (comment). It is now retargeted to 26_ux since the executors were refactored there.

Use case, as Paul described: a custom executor injecting application-level dependencies (connection pools, API clients) as keyword arguments into every job, without wrapping each job function.

On 26_ux all built-in executors go through prepare_job_for_execution, which resolved the function via a private static helper. The public get_function_and_kwargs was a thin wrapper hardcoded to Executor, so overriding it in a subclass had no effect.

What changes

• get_function_and_kwargs becomes a classmethod holding the resolution logic, and prepare_job_for_execution calls cls.get_function_and_kwargs. Its docstring names it as the override point and carries the warning about where it runs (event loop, thread, sub-interpreter, child process).
• The job-context detection used by co-operative time limits now checks the resolved kwargs for the job instance, so an override cannot break it by accident.
• Tests: a ThreadedExecutor subclass injects a kwarg at the prepare_job_for_execution level, and an end-to-end test checks the injection through a real worker on the process, threaded, async and sub-interpreter executors.
• Changelog entry under 0.26.0.

[luca-montaigut]

This one was originally intentional, as significant care has to be taken when crossing process/thread/sub-interpreter barriers. But your use case is definitely valid and this is definitely safe in the async executor. I'll give the others a closer review.

On 26_ux this concern is addressed by your own refactor: job_wrapper and prepare_job_for_execution are classmethods, and every executor submits cls.job_wrapper to its pool. For the process and sub-interpreter pools that bound method is pickled by reference and re-imported on the other side, so an override of get_function_and_kwargs runs where the job runs, never in the worker. Nothing crosses the boundary except the executor's importable name, which is the same rule as on_initialize_worker.

The updated PR only routes the resolution through the public name instead of the private helper. The docstring now spells out where the method runs and what that implies, and test_injected_kwargs_reach_jobs_on_every_executor checks the injection end to end on the process, threaded, async and sub-interpreter executors.

[TkTech]

Merged in #58, with one change to the docstring (async and threaded executors can generally share objects)