Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds after_job_end hook #355

Merged
merged 2 commits into from
Dec 1, 2022
Merged

Conversation

AngellusMortis
Copy link
Contributor

@AngellusMortis AngellusMortis commented Nov 2, 2022

Adds an additional hook for after the job completes. Useful for doing something after the results for the job has been recorded.

My specific use case is I am adding New Relic integration into arq for our projects. Being able to record the success/failure, queue time, etc. of a job directly to New Relic is very useful.

@codecov
Copy link

codecov bot commented Nov 2, 2022

Codecov Report

Merging #355 (3564376) into main (9ce2fd6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #355   +/-   ##
=======================================
  Coverage   98.80%   98.80%           
=======================================
  Files           9        9           
  Lines         917      920    +3     
  Branches      166      167    +1     
=======================================
+ Hits          906      909    +3     
  Misses          6        6           
  Partials        5        5           
Impacted Files Coverage Δ
arq/worker.py 99.56% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ce2fd6...3564376. Read the comment docs.

@JonasKs
Copy link
Sponsor Collaborator

JonasKs commented Nov 2, 2022

What happens if this task has an exception? on_job_end failure acts like the task itself failed, and I don’t believe that’s what you’d like in this scenario.

@AngellusMortis
Copy link
Contributor Author

AngellusMortis commented Nov 2, 2022

on_job_end and on_job_start do not handle exceptions either. I think all 3 of them should, but I think that is out of scope of this PR unless we wanted to apply it to all of the hooks.

@AngellusMortis
Copy link
Contributor Author

AngellusMortis commented Nov 2, 2022

Probably more specifically, it would be nice to see something like this (abbreviated, but hopefully you understand what I mean):

async def run_job(...):
    ....
    
    async def job_wrapper():
        await on_job_start(ctx)
        await function.coroutine(ctx, *args, **kwargs)
        await on_job_end(ctx)
     
    await asyncio.shield(before_job_start(ctx))
    await self.loop.create_task(job_wrapper())
    await await asyncio.shield(self.finish_job())
    await asyncio.shield(after_job_end(ctx))

This would allow for:

  • optional before/after hooks for reporting/etc. that can fail without affecting the outcome of the task
  • a required before/after job hooks that are required to succeed for the job to succeed (inject/clean up dependencies)

@samuelcolvin
Copy link
Owner

@AngellusMortis LGTM, could you resolve conflicts.

@AngellusMortis
Copy link
Contributor Author

Fixed.

@samuelcolvin samuelcolvin merged commit e9452c5 into samuelcolvin:main Dec 1, 2022
@samuelcolvin
Copy link
Owner

thanks so much.

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.

None yet

3 participants