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.

Sign up for GitHub

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

Jump to bottom

Fix race condition when pending Promise was resolved in other thread #89

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix race condition when pending Promise was resolved in other thread #89

wants to merge 3 commits into from

Conversation

alex-verve
Copy link

@alex-verve alex-verve commented Jul 15, 2020 •
edited
Loading

I had to pin pytest* deps to make tests working again (new versions of pytest-cov and pytest-asyncio are not supported by latest pytest version for pypy)

Fixes #87

Test failure (without fix) - https://travis-ci.org/github/syrusakbary/promise/builds/708825617
Test passed here (with fix) - https://travis-ci.org/github/syrusakbary/promise/builds/708827378

@alex-verve alex-verve marked this pull request as ready for review July 15, 2020 23:00
@alex-verve alex-verve force-pushed the master branch 7 times, most recently from 1c918aa to e4235ad Compare July 16, 2020 13:19
Last version of pytest for pypy is 4.6.11;
5cb778d 
Also latest version pytest-cov and pytest-asyncio are not compatible with latest pytest.

https://travis-ci.org/github/syrusakbary/promise/jobs/708528307

 Could not find a version that satisfies the requirement pytest>=5.4.3 (from promise==2.3) (from versions: 2.0.0, ......... 4.6.10, 4.6.11)

https://travis-ci.org/github/syrusakbary/promise/jobs/708523881

E   AttributeError: type object 'Function' has no attribute 'from_parent'

https://travis-ci.org/github/syrusakbary/promise/jobs/708735234

pytest-cov 2.10.0 has requirement pytest>=4.6, but you'll have pytest 4.3.1 which is incompatible.
@alex-verve alex-verve force-pushed the master branch 5 times, most recently from 137b395 to e975419 Compare July 16, 2020 14:57
@alex-verve alex-verve mentioned this pull request Jul 16, 2020
Open
@alex-verve alex-verve changed the title Fix race condition when waiting on Promise resolved in other thread Fix race condition when pending Promise was resolved in other thread Jul 17, 2020
@alex-verve
Copy link
Author

There is at least one more race-condition in PromisList

  File "/var/task/promise/promise.py", line 97, in try_catch
    return (handler(*args, **kwargs), None)
, 
  File "/var/task/graphql/execution/executor.py", line 260, in <lambda>
    resolved
, 
  File "/var/task/graphql/execution/executor.py", line 288, in complete_value
    return complete_object_value(exe_context, return_type, field_asts, info, result)
, 
  File "/var/task/graphql/execution/executor.py", line 382, in complete_object_value
    return execute_fields(exe_context, return_type, result, subfield_asts)
, 
  File "/var/task/graphql/execution/executor.py", line 146, in execute_fields
    return promise_for_dict(final_results)
, 
  File "/var/task/promise/promise.py", line 816, in for_dict
    return cls.all(m.values()).then(handle_success)
, 
  File "/var/task/promise/promise.py", line 796, in all
    return PromiseList(promises, promise_class=cls).promise
, 
  File "/var/task/promise/promise_list.py", line 40, in __init__
    self._init(values)  # type: ignore
, 
  File "/var/task/promise/promise_list.py", line 72, in _init
    self._iterate(values)
, 
  File "/var/task/promise/promise_list.py", line 95, in _iterate
    None,

@Zahlii
Copy link

Zahlii commented Sep 8, 2020

Is there any update on this or any way to help out? This is a critical issue since we are using Promise for job scheduling in production, and every once in a while this error comes around and forces us to re-run

jkimbo added a commit to strawberry-graphql/strawberry that referenced this pull request Jan 8, 2021
@jkimbo
Add Promise library
90063b5 
Also incorporate fix: syrusakbary/promise#89
patrick91 pushed a commit to strawberry-graphql/strawberry that referenced this pull request Feb 8, 2021
@jkimbo @patrick91
Add Promise library
c13727e 
Also incorporate fix: syrusakbary/promise#89
patrick91 pushed a commit to strawberry-graphql/strawberry that referenced this pull request Apr 1, 2021
@jkimbo @patrick91
Add Promise library
40c35af 
Also incorporate fix: syrusakbary/promise#89
jkimbo added a commit to strawberry-graphql/strawberry that referenced this pull request Jun 15, 2021
@jkimbo
Add Promise library
8238da3 
Also incorporate fix: syrusakbary/promise#89
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Race condition when waiting on Promise resolved in other thread
2 participants
@alex-verve @Zahlii