Fixes build for PyPy3 by mjhoffman65 · Pull Request #6635 · celery/celery

mjhoffman65 · 2021-02-18T20:20:28Z

@auvipy, I took at stab at fixing the PyPy failures. My first step is to try to install all the requirements and recreate the unit test failure in the docker environment. I ran into issues installing the python requirements and am wondering if more work needs to be done before we can expect celery to run with PyPy. Looks like at least a couple of the dependencies do not work with PyPy since they rely on a C backend.

couchbase: https://docs.couchbase.com/python-sdk/current/hello-world/start-using-sdk.html#pypy-support
ephem: celery/django-celery-beat#69 (comment) (Note we may be able to replace this with skyfield. See here for more.)

This PR illustrates the same error I'm getting in the docker environment: https://github.com/celery/celery/runs/1930241932?check_suite_focus=true

How should I move forward?

auvipy · 2021-02-19T03:11:34Z

I think we should stop couchbase testing and eventually remove it from the build. I am not sure how much popular it is.

mjhoffman65 · 2021-02-22T18:23:01Z

We now see actual errors.
Are you working on fixing those?

Yep! Working now to fix them.

mjhoffman65 · 2021-02-22T18:41:52Z

@auvipy @thedrow This is ready for review. I have some in-progress Dockerfile changes making it easy to run the unit tests with pypy3 via docker as well, but that can go separately.

lgtm-com · 2021-02-22T18:44:22Z

This pull request introduces 1 alert and fixes 2 when merging c0ca554 into 3384937 - view on LGTM.com

new alerts:

1 for Unused import

fixed alerts:

1 for Non-exception in 'except' clause
1 for Module is imported with 'import' and 'import from'

mjhoffman65 · 2021-02-22T21:48:50Z

What about that event is a boolean failure that pops up every now and then?
@auvipy You know what I'm talking about right? Can you find a build log where that happens?
@mjhoffman65 If you repeatedly run the test suite multiple times, does it always pass?

@thedrow I've ran it 4 times in docker and so far it has always passed. I'm not sure how to trigger reruns with the github actions, but I haven't seen that error pop up yet. I'm assuming it's this one: #6489

mjhoffman65 · 2021-02-23T20:41:08Z

Was able to recreate #6489. Will reopen after I fix it.

lgtm-com · 2021-03-02T14:29:24Z

This pull request introduces 1 alert and fixes 2 when merging 3712de6 into cfa1b41 - view on LGTM.com

new alerts:

1 for Unused import

fixed alerts:

1 for Non-exception in 'except' clause
1 for Module is imported with 'import' and 'import from'

maybe-sybr · 2021-05-07T01:22:39Z

Those dockerfile changes @mjhoffman65 mentioned might make life easier, but I don't see a ref to them. In any case, I've got a pypy3 interpreter on my box so I'll see if I can clean this up.

maybe-sybr · 2021-05-07T02:14:09Z

Rebased this to hopefully fix the CI failures on that delivery info test, also with the correct fix for #6489

Edit: Annoyingly couchbase now doesn't appear to be ~~getting installed in the github jobs?~~ configured properly?

And there's another test breaking under pypy3 in test_events.py :(

We could cherry pick 651a0ed by itself but I'm not confident enough to spend more time on this in the lead up to 5.1.

lgtm-com · 2021-05-07T04:02:17Z

This pull request fixes 3 alerts when merging 82d7ad3 into 426a8f9 - view on LGTM.com

fixed alerts:

1 for Non-exception in 'except' clause
1 for Unused import
1 for Module is imported with 'import' and 'import from'

maybe-sybr · 2021-05-09T23:38:07Z

It's still broken :( I de-drafted this thinking it would run through and go green and now can't work out how to put it back 😬

Edit: Found it. It was a text link under the reviewers rather than a nice button...

maybe-sybr · 2021-05-24T04:00:53Z

Freshened this on top of master now that 5.1 is released. I've also made some changes to the workflow to run unit tests using tox. I think that might fix up some of the weirdness around the missing dependencies. We'll see...

lgtm-com · 2021-05-24T04:39:15Z

This pull request fixes 2 alerts when merging 4dd514e into 025bad6 - view on LGTM.com

fixed alerts:

1 for Non-exception in 'except' clause
1 for Module is imported with 'import' and 'import from'

Mocking `datetime.datetime` was causing an issue with `datetime.utcnow()`. This mock doesn't appear to be needed. See https://github.com/celery/celery/pull/6635/checks?check_run_id=1944166896.

Fixes celery#6489

maybe-sybr · 2021-06-09T06:36:17Z

Freshened on master and also with a few extra fixes for the same kind of Thread._is_stopped shadowing in the bgThread class which is probably broken in a race-y way similar to Timer but only touched in two tests so we probably never noticed that it was different to the timer failure.

I'm hoping this will go green now, with the fix from #6806 pulled in via the rebase as well.

codecov · 2021-06-09T06:43:35Z

Codecov Report

Merging #6635 (1ce3b20) into master (d9d8250) will increase coverage by 5.12%.
The diff coverage is 100.00%.

❗ Current head 1ce3b20 differs from pull request most recent head f486c0d. Consider uploading reports for the commit f486c0d to get more accurate results

@@            Coverage Diff             @@
##           master    #6635      +/-   ##
==========================================
+ Coverage   70.70%   75.82%   +5.12%     
==========================================
  Files         138      138              
  Lines       16603    16603              
  Branches     2092     2092              
==========================================
+ Hits        11739    12590     +851     
+ Misses       4668     3797     -871     
- Partials      196      216      +20

Flag	Coverage Δ
unittests	`75.82% <100.00%> (+5.12%)`	⬆️

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

Impacted Files	Coverage Δ
celery/backends/redis.py	`92.17% <ø> (+8.93%)`	⬆️
celery/bin/base.py	`0.00% <ø> (ø)`
celery/concurrency/__init__.py	`100.00% <ø> (ø)`
celery/exceptions.py	`32.60% <ø> (ø)`
celery/worker/consumer/consumer.py	`93.57% <ø> (ø)`
celery/canvas.py	`94.82% <100.00%> (ø)`
celery/utils/threads.py	`65.43% <100.00%> (ø)`
celery/utils/timer2.py	`98.95% <100.00%> (ø)`
celery/app/trace.py	`98.60% <0.00%> (+0.27%)`	⬆️
celery/result.py	`58.01% <0.00%> (+0.40%)`	⬆️
... and 22 more

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 d9d8250...f486c0d. Read the comment docs.

maybe-sybr · 2021-06-09T06:54:17Z

Huh, weird, it's trying to run flake8 under action environments without python 3.9 installed :/

Edit: Oh 0295d7a did that. Probably no point in linting in each of the build jobs though, that's kind of weird

lgtm-com · 2021-06-09T07:05:54Z

This pull request introduces 1 alert and fixes 1 when merging a3b2af7 into d9d8250 - view on LGTM.com

new alerts:

1 for Non-exception in 'except' clause

fixed alerts:

1 for Module is imported with 'import' and 'import from'

Also fix some flakes which may have been added by some other autoformatter in celery#6804. The 4 space non-visual-indentation should keep most formatters fairly happy.

lgtm-com · 2021-06-09T07:58:34Z

This pull request fixes 1 alert when merging f486c0d into d9d8250 - view on LGTM.com

fixed alerts:

1 for Module is imported with 'import' and 'import from'

* installs packages the same way docker does * removes couchbase dependency for PyPy * removes ephem dependency for PyPy * fixes mongo unit tests for PyPy3 Mocking `datetime.datetime` was causing an issue with `datetime.utcnow()`. This mock doesn't appear to be needed. See https://github.com/celery/celery/pull/6635/checks?check_run_id=1944166896. * fix: Avoid shadowing `Thread` attributes Fixes celery#6489 * ci: Install default deps for pypy3 toxenvs * ci: Run unit tests with `tox` * ci: Lint source in separate action using `tox` * ci: Redent codecov action * test: Rework some mocking in `test_platforms.py` Also fix some flakes which may have been added by some other autoformatter in celery#6804. The 4 space non-visual-indentation should keep most formatters fairly happy. * style: Fix some flakes Co-authored-by: maybe-sybr <58414429+maybe-sybr@users.noreply.github.com>

mjhoffman65 changed the title ~~installs packages the same way docker does~~ PyPy Compatibility Feb 18, 2021

mjhoffman65 changed the title ~~PyPy Compatibility~~ PyPy Compatibility? Feb 18, 2021

mjhoffman65 mentioned this pull request Feb 18, 2021

start chord header tasks as soon as possible #6576

Merged

mjhoffman65 force-pushed the fix_pypy_check branch from 968dd76 to 5b0a979 Compare February 21, 2021 01:30

mjhoffman65 force-pushed the fix_pypy_check branch from 5b0a979 to c0ca554 Compare February 22, 2021 18:28

mjhoffman65 changed the title ~~PyPy Compatibility?~~ Fixes build for PyPy3 Feb 22, 2021

mjhoffman65 marked this pull request as ready for review February 22, 2021 18:41

mjhoffman65 closed this Feb 23, 2021

auvipy reopened this Feb 24, 2021

auvipy added this to the 5.1.0 milestone Feb 24, 2021

mjhoffman65 force-pushed the fix_pypy_check branch from c0ca554 to 3712de6 Compare March 2, 2021 13:56

thedrow assigned mjhoffman65 Apr 7, 2021

maybe-sybr self-assigned this May 7, 2021

maybe-sybr mentioned this pull request May 7, 2021

pypy3 unit tests sometimes fail due to an attribute we expect to be an Event being a bool instead #6489

Closed

18 tasks

maybe-sybr force-pushed the fix_pypy_check branch from 2d65713 to 206f5aa Compare May 7, 2021 02:13

maybe-sybr force-pushed the fix_pypy_check branch from 206f5aa to 651a0ed Compare May 7, 2021 02:14

maybe-sybr marked this pull request as ready for review May 7, 2021 02:51

maybe-sybr marked this pull request as draft May 9, 2021 23:38

maybe-sybr force-pushed the fix_pypy_check branch from 82d7ad3 to 19a4c33 Compare May 24, 2021 03:59

mjhoffman65 and others added 7 commits June 9, 2021 15:31

installs packages the same way docker does

8c0d73c

removes couchbase dependency for PyPy

002b805

removes ephem dependency for PyPy

8df0445

fixes mongo unit tests for PyPy3

822b044

Mocking `datetime.datetime` was causing an issue with `datetime.utcnow()`. This mock doesn't appear to be needed. See https://github.com/celery/celery/pull/6635/checks?check_run_id=1944166896.

fix: Avoid shadowing Thread attributes

0bd8a0d

Fixes celery#6489

ci: Install default deps for pypy3 toxenvs

f04d9b6

ci: Run unit tests with tox

7737a4b

maybe-sybr force-pushed the fix_pypy_check branch from 4dd514e to a3b2af7 Compare June 9, 2021 06:34

maybe-sybr force-pushed the fix_pypy_check branch 2 times, most recently from 60a8122 to 1ce3b20 Compare June 9, 2021 07:41

maybe-sybr added 4 commits June 9, 2021 17:42

ci: Lint source in separate action using tox

68da298

ci: Redent codecov action

8887bb9

test: Rework some mocking in test_platforms.py

1b1c9e9

Also fix some flakes which may have been added by some other autoformatter in celery#6804. The 4 space non-visual-indentation should keep most formatters fairly happy.

style: Fix some flakes

f486c0d

maybe-sybr force-pushed the fix_pypy_check branch from 1ce3b20 to f486c0d Compare June 9, 2021 07:43

maybe-sybr marked this pull request as ready for review June 9, 2021 10:14

maybe-sybr requested a review from thedrow June 9, 2021 10:14

auvipy approved these changes Jun 9, 2021

View reviewed changes

thedrow merged commit 117cd9c into celery:master Jun 27, 2021

maybe-sybr mentioned this pull request Jul 21, 2021

Fix UnboundLocalError when running celery worker in foreground #6833

Merged

Uh oh!

Conversation

mjhoffman65 commented Feb 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

auvipy commented Feb 19, 2021

Uh oh!

mjhoffman65 commented Feb 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjhoffman65 commented Feb 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgtm-com bot commented Feb 22, 2021

Uh oh!

mjhoffman65 commented Feb 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjhoffman65 commented Feb 23, 2021

Uh oh!

lgtm-com bot commented Mar 2, 2021

Uh oh!

maybe-sybr commented May 7, 2021

Uh oh!

maybe-sybr commented May 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgtm-com bot commented May 7, 2021

Uh oh!

maybe-sybr commented May 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maybe-sybr commented May 24, 2021

Uh oh!

lgtm-com bot commented May 24, 2021

Uh oh!

maybe-sybr commented Jun 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jun 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

maybe-sybr commented Jun 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgtm-com bot commented Jun 9, 2021

Uh oh!

lgtm-com bot commented Jun 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mjhoffman65 commented Feb 18, 2021 •

edited

Loading

mjhoffman65 commented Feb 22, 2021 •

edited

Loading

mjhoffman65 commented Feb 22, 2021 •

edited

Loading

mjhoffman65 commented Feb 22, 2021 •

edited

Loading

maybe-sybr commented May 7, 2021 •

edited

Loading

maybe-sybr commented May 9, 2021 •

edited

Loading

maybe-sybr commented Jun 9, 2021 •

edited

Loading

codecov bot commented Jun 9, 2021 •

edited

Loading

maybe-sybr commented Jun 9, 2021 •

edited

Loading