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

Fix redis pipeline created and not used #374

Merged
merged 1 commit into from
Dec 14, 2022

Conversation

iamlikeme
Copy link
Contributor

Fixes a mistake introduced in #364 where a Redis pipeline was created but not used (ArqRedis is used directly).
The pipeline was there to prevent race conditions, so it should be used.

@codecov
Copy link

codecov bot commented Dec 11, 2022

Codecov Report

Merging #374 (18e57ea) into main (430df07) will increase coverage by 0.12%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #374      +/-   ##
==========================================
+ Coverage   98.64%   98.76%   +0.12%     
==========================================
  Files           9       11       +2     
  Lines         957     1050      +93     
  Branches      179      198      +19     
==========================================
+ Hits          944     1037      +93     
  Misses          6        6              
  Partials        7        7              
Impacted Files Coverage Δ
arq/jobs.py 98.15% <100.00%> (+0.01%) ⬆️
arq/__init__.py 100.00% <0.00%> (ø)
arq/cron.py 100.00% <0.00%> (ø)

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 430df07...18e57ea. Read the comment docs.

@samuelcolvin samuelcolvin merged commit e6ff090 into python-arq:main Dec 14, 2022
@samuelcolvin
Copy link
Member

LGTM, thanks so much.

@iamlikeme iamlikeme deleted the piotr/fix-unused-pipeline branch December 18, 2022 20:20
@arturzx
Copy link

arturzx commented Apr 2, 2023

Hello, it is chance for new release with this fix? I updated to latest 0.25 and this update leads to random fake ResultNotFound errors (proabably race condition because I use 0.01 poll_delay).

@samuelcolvin
Copy link
Member

@JonasKs do you think we can do a new release?

I can help with it on Tuesday hopefully. (Once we have the first alpha of pydantic v2 out)

@JonasKs
Copy link
Collaborator

JonasKs commented Apr 2, 2023

Yes! 😊 I'll look over all PRs tomorrow and see if there's some issues I can resolve.

I'm available all week. 😊

@samuelcolvin
Copy link
Member

Amazing, thank you.

@arturzx
Copy link

arturzx commented May 5, 2023

Yes! blush I'll look over all PRs tomorrow and see if there's some issues I can resolve.

I'm available all week. blush

Hello, any progress on this? :-) Thank you!

@blakehawkins
Copy link

Hi friends, we're also hitting spurious/transient arq.jobs.ResultNotFound errors in arq 0.25

Wondering if there's a recommended workaround and/or rough timeline for the next release fixing this?

All the best

@m-kuhn
Copy link

m-kuhn commented May 31, 2023

FWIW, I added the following in my requirements.txt to avoid the arq.jobs.ResultNotFound errors:

arq @ git+https://github.com/samuelcolvin/arq@9109c2e59d2b13fa59d246da03d19d7844a6fa19

A release would be appreciated

@m-kuhn
Copy link

m-kuhn commented May 31, 2023

Hello, it is chance for new release with this fix? I updated to latest 0.25 and this update leads to random fake ResultNotFound errors (proabably race condition because I use 0.01 poll_delay).

It seems you are looking for a quick roundtrip. I find myself in a similar situation and I am wondering if there would be a more resource friendly solution than this "spin loop", using pubsub (possibly with keyspace notifications) on one end and using one of the blocking redis functions (block until a job is is waiting).

@KShah707
Copy link

FWIW, I added the following in my requirements.txt to avoid the arq.jobs.ResultNotFound errors:

arq @ git+https://github.com/samuelcolvin/arq@9109c2e59d2b13fa59d246da03d19d7844a6fa19

A release would be appreciated

+1. A new release with this fix would be appreciated.

@aviau
Copy link

aviau commented Mar 22, 2024

Any plan on a bugfix release @samuelcolvin ?

I think we are also getting the JobNotFound error after upgrading to 0.25.

@samuelcolvin
Copy link
Member

v0.26.0b1 is released, please try it, I'll release v0.26 at the end of the week, see #441.

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.

8 participants