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

Release 0.12.0 #94

Merged
merged 37 commits into from
Jan 8, 2021
Merged

Release 0.12.0 #94

merged 37 commits into from
Jan 8, 2021

Conversation

shamrin
Copy link
Contributor

@shamrin shamrin commented Dec 31, 2020

Release notes screenshot

TODO

Fixes #93

@shamrin shamrin marked this pull request as ready for review January 1, 2021 21:34
[[tool.towncrier.type]]
directory = "misc"
name = "Miscellaneous"
showcontent = true
Copy link
Contributor Author

@shamrin shamrin Jan 1, 2021

Choose a reason for hiding this comment

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

The showcontent = true line above is the whole reason for changing towncrier config. Otherwise, misc notes are missing their content in history.rst. I did it the same way as in trio: https://github.com/python-trio/trio/blob/master/pyproject.toml

@@ -352,7 +228,7 @@ async def run_asyncio_loop(nursery, *, task_status=trio.TASK_STATUS_IGNORED):
# Trigger KeyboardInterrupt that should propagate accross the coroutines
signal.pthread_kill(threading.get_ident(), signal.SIGINT)


@pytest.mark.xfail(reason="https://github.com/python-trio/trio-asyncio/issues/95", raises=RuntimeError)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change makes CI green. See also #95.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for diagnosing this. I just uploaded #96 which appears to fix it. Probably worth incorporating into the release?

@smurfix
Copy link
Collaborator

smurfix commented Jan 2, 2021

The test error is from Python 3.10 which dropped a heap of loop=loop parameters from Future, Event et al.

I'm building a minimal patch; more extensive de-loop-argument-ization (in trio-asyncioproper) makes sense but requires deprecation warnings.

required for for Python 3.10+
@codecov
Copy link

codecov bot commented Jan 2, 2021

Codecov Report

Merging #94 (a18c2b4) into master (b93c320) will increase coverage by 1.16%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
+ Coverage   80.64%   81.81%   +1.16%     
==========================================
  Files          11       11              
  Lines        1240     1193      -47     
  Branches      172      172              
==========================================
- Hits         1000      976      -24     
+ Misses        170      154      -16     
+ Partials       70       63       -7     
Impacted Files Coverage Δ
trio_asyncio/__init__.py 100.00% <ø> (ø)
trio_asyncio/_loop.py 79.32% <50.00%> (+0.63%) ⬆️
trio_asyncio/_adapter.py 81.19% <100.00%> (+1.03%) ⬆️
trio_asyncio/_base.py 84.14% <100.00%> (-0.64%) ⬇️
trio_asyncio/_util.py 84.09% <100.00%> (ø)
trio_asyncio/_version.py 100.00% <100.00%> (ø)
trio_asyncio/_deprecate.py 100.00% <0.00%> (+19.44%) ⬆️

@shamrin
Copy link
Contributor Author

shamrin commented Jan 2, 2021

@smurfix Thank you! I considered doing this change myself, but I thought it would make trio-asyncio Python 3.8+ only. Test results are proving me wrong :)

@shamrin
Copy link
Contributor Author

shamrin commented Jan 3, 2021

Code coverage got lower. Most of it is due to _deprecate module. We use much less of it now, due to removed deprecations. Options (ordered by increasing difficulty):

  • copy test_deprecate from Trio and adapt it (easiest)
  • remove unused functionality from _deprecate module
  • extract _deprecate to its own package and use it in both trio and trio-asyncio

Update: I did port test_deprecate from Trio and codecov is finally happy. 👇

@@ -344,7 +342,7 @@ async def _waitpid(self, pid, callback, *args):

def add_child_handler(self, pid, callback, *args):
"""Add a callback to run when a child process terminates."""
h = self._loop.run_trio(self._waitpid, pid, callback, *args)
h = self._loop.trio_as_future(self._waitpid, pid, callback, *args)
Copy link
Contributor Author

@shamrin shamrin Jan 3, 2021

Choose a reason for hiding this comment

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

Codecov is not happy about this line. It was never covered by tests before, and I have no idea how to test it now. However, the change is equivalent to what it was previously:

    @deprecated("0.10.0", issue=38, instead="trio_as_aio(proc)(*args)")
    def run_trio(self, proc, *args):
        return self.trio_as_future(proc, *args)

docs/source/history.rst Outdated Show resolved Hide resolved
docs/source/history.rst Outdated Show resolved Hide resolved
docs/source/history.rst Outdated Show resolved Hide resolved
@@ -352,7 +228,7 @@ async def run_asyncio_loop(nursery, *, task_status=trio.TASK_STATUS_IGNORED):
# Trigger KeyboardInterrupt that should propagate accross the coroutines
signal.pthread_kill(threading.get_ident(), signal.SIGINT)


@pytest.mark.xfail(reason="https://github.com/python-trio/trio-asyncio/issues/95", raises=RuntimeError)
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for diagnosing this. I just uploaded #96 which appears to fix it. Probably worth incorporating into the release?

@shamrin
Copy link
Contributor Author

shamrin commented Jan 4, 2021

@oremanj:

I just uploaded #96 which appears to fix it. Probably worth incorporating into the release?

Awesome, thank you! Yes, let's postpone the release until we get #96 merged.

(In the mean time, I've made the fixes you suggested.)

shamrin added a commit to oremanj/trio-asyncio that referenced this pull request Jan 5, 2021
This reverts (cherry-picked) commits 8930bf2 and d1051b0.

We will do the de-`loop=`-ification in PR python-trio#94.
@oremanj
Copy link
Member

oremanj commented Jan 6, 2021

This LGTM. I merged #96 so once you incorporate that (and #99 maybe?) I think it's ready.

@oremanj
Copy link
Member

oremanj commented Jan 7, 2021

Interesting that this PR hasn't had any of the problems that the Mac builds have been having on other PRs. Those all seem to have failed around test_cancel_sleep_depr in test_misc.py, which is now gone. But I can't for the life of me figure out why test_cancel_sleep_depr would hang if the equivalent test_cancel_sleep works fine. Gremlins!

@shamrin
Copy link
Contributor Author

shamrin commented Jan 7, 2021

@oremanj Now it did stuck at test_cancel_sleep. And I did find a way to replicate the issue locally. Issue coming soon!

Update: here is the issue - #103. Should we fix this deadlock before the release? Opinions?

No need for `raise Exception` because we already assert the flag.

fixes python-trio#103
@shamrin
Copy link
Contributor Author

shamrin commented Jan 8, 2021

"Fixed" the #103 deadlock in test_cancel_sleep by removing unnecessary raise Exception. The test would now fail with AssertionError rather than deadlock. I've also fiddled with timeout length to lower failure probability: 970d81c.

I think the underlying cause of deadlock is #104. No need to fix it for this release, because the problem was in v0.11.0 as well.

@shamrin shamrin merged commit fe01773 into python-trio:master Jan 8, 2021
@shamrin shamrin deleted the release-0.12.0 branch January 8, 2021 01:05
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.

Time for a new trio-asyncio release?
4 participants