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 up Python 3.8 loop argument warnings #246

Merged
merged 8 commits into from Apr 23, 2020

Conversation

achimnol
Copy link
Member

@achimnol achimnol commented Apr 4, 2020

What do these changes do?

  • Remove all loop=self.loop expressions.
  • Rely on the currently running loop in the constructor of Queue.
  • Assuming that janus.Queue objects are created in the functions or coroutines called by the event loop, rewrite most test cases to be async.
    • No longer manage the event loop lifecycles by ourselves.
    • Adopt pytest-asyncio to seamlessly run test cases in an event loop.
  • Add missing .close() / .wait_closed() calls to the end of many test cases to ensure proper termination of the queues.
  • Insert asyncio.sleep(0) in the wait_closed() method so that all task-done callbacks for tasks spawned by _notify_async_not_empty(), _notify_async_not_full() internal methods are properly awaited.
    This eliminates hundreds of resource warnings after finishing the test suite and loop termination after using .join() APIs.
  • Ensure dropping of Python 3.3/3.4 in CI configs.
  • Add Python 3.7 and 3.8 to CI configs.

Are there changes in behavior for the user?

Users are no longer able to create janus.Queue instances in the codes running outside event loops (even when they have references to an existing event loop running somewhere else).

Related issue number

#229

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add a new news fragment into the CHANGES folder
    • There is no CHANGES folder, so I've added a bullet in CHANGES.rst directly.

* Remove all "loop=self.loop" expressions.

* Rely on the currently running loop in the constructor of Queue.

* Assuming that janus.Queue objects are created in the functions or
  coroutines called by the event loop, rewrite most test cases to
  be async.

  - No longer manage the event loop lifecycles by ourselves.

  - Adopt pytest-asyncio to seamlessly run test cases in an event loop.

* Add missing .close() / .wait_closed() calls to the end of many test
  cases to ensure proper termination of the queues.

* Insert asyncio.sleep(0) in the wait_closed() method so that all
  task-done callbacks for tasks spawned by _notify_async_not_empty(),
  _notify_async_not_full() internal methods are properly awaited.
  This eliminates hundreds of resource warnings after finishing the test
  suite.

* Ensure dropping of Python 3.3/3.4 in CI configs.

* Add Python 3.7 and 3.8 to CI configs.
@codecov
Copy link

codecov bot commented Apr 4, 2020

Codecov Report

Merging #246 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #246   +/-   ##
=======================================
  Coverage   99.67%   99.67%           
=======================================
  Files           1        1           
  Lines         308      309    +1     
  Branches       40       41    +1     
=======================================
+ Hits          307      308    +1     
  Partials        1        1           
Impacted Files Coverage Δ
janus/__init__.py 99.67% <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 03d1b36...dd1517a. Read the comment docs.

@achimnol
Copy link
Member Author

@asvetlov Just a poke, please have a look at this PR.

Copy link
Member

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!
Very impressive work, thanks!

@asvetlov asvetlov merged commit ec8592b into aio-libs:master Apr 23, 2020
@achimnol achimnol deleted the fix/py38-loop-warnings branch April 23, 2020 13:10
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

2 participants