Re: [PATCH 2/2] python-cffi: returned OwnedMessage objects from Message.replies
Floris Bruynooghe writes: > On Sun 09 Jan 2022 at 09:26 -0400, David Bremner wrote: >> One thing I would like to think >> about is the length of time it takes to run the pytests. It is not >> currently the bottleneck in running the parallel tests for me, but it is >> among the slower T*.sh. So it might be nice at some point to split into >> several invocations for pytest, assuming multiple invocations of pytest >> can run in parallel. > > There is the pytest-xdist plugin which is widely used and probably will > work fine on the notmuch testsuite (I even contributed to the plugin at > some point). It will split the workload between however many processes > you choose, running them in parallel. It sounds like it has potential, but I'm not sure how it would interact with the existing scheme controlled by make. I suspect we'd end up with too many processes/threads in total. d ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH 2/2] python-cffi: returned OwnedMessage objects from Message.replies
On Sun 09 Jan 2022 at 09:26 -0400, David Bremner wrote: > One thing I would like to think > about is the length of time it takes to run the pytests. It is not > currently the bottleneck in running the parallel tests for me, but it is > among the slower T*.sh. So it might be nice at some point to split into > several invocations for pytest, assuming multiple invocations of pytest > can run in parallel. There is the pytest-xdist plugin which is widely used and probably will work fine on the notmuch testsuite (I even contributed to the plugin at some point). It will split the workload between however many processes you choose, running them in parallel. ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH 2/2] python-cffi: returned OwnedMessage objects from Message.replies
Floris Bruynooghe writes: > oh, nice debugging! And yes, this seems like the right fix, glad the > mechanism was at least already in place. > > With respect to the docs, they seem clear enough. Probably I missed > this or I simply didn't realise that the replies iter is basically a > thread owning it. > Thanks for the review. > Only thing I'd do different is write a pytest test for this as well, but > than I wouldn't have written the notmuch tests. So I don't think this > matters that much. We can always add a pytest test later. One thing I would like to think about is the length of time it takes to run the pytests. It is not currently the bottleneck in running the parallel tests for me, but it is among the slower T*.sh. So it might be nice at some point to split into several invocations for pytest, assuming multiple invocations of pytest can run in parallel. d ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [PATCH 2/2] python-cffi: returned OwnedMessage objects from Message.replies
On Sat 08 Jan 2022 at 10:03 -0400, David Bremner wrote:
> If we return regular Message objects, python will try to destroy them,
> and the underlying notmuch object, causing e.g. the crash [1].
>
> [1]: id:87sfu6utxg@tethera.net
> ---
> bindings/python-cffi/notmuch2/_message.py | 4 ++--
> test/T392-python-cffi-notmuch.sh | 2 --
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/bindings/python-cffi/notmuch2/_message.py
> b/bindings/python-cffi/notmuch2/_message.py
> index a460d8c1..aa1cb875 100644
> --- a/bindings/python-cffi/notmuch2/_message.py
> +++ b/bindings/python-cffi/notmuch2/_message.py
> @@ -371,14 +371,14 @@ class Message(base.NotmuchObject):
> This method will only work if the message was created from a
> thread. Otherwise it will yield no results.
>
> -:returns: An iterator yielding :class:`Message` instances.
> +:returns: An iterator yielding :class:`OwnedMessage` instances.
> :rtype: MessageIter
> """
> # The notmuch_messages_valid call accepts NULL and this will
> # become an empty iterator, raising StopIteration immediately.
> # Hence no return value checking here.
> msgs_p = capi.lib.notmuch_message_get_replies(self._msg_p)
> -return MessageIter(self, msgs_p, db=self._db)
> +return MessageIter(self, msgs_p, db=self._db, msg_cls=OwnedMessage)
>
> def __hash__(self):
> return hash(self.messageid)
> diff --git a/test/T392-python-cffi-notmuch.sh
> b/test/T392-python-cffi-notmuch.sh
> index 50012c55..15c8fc6b 100755
> --- a/test/T392-python-cffi-notmuch.sh
> +++ b/test/T392-python-cffi-notmuch.sh
> @@ -24,13 +24,11 @@ show_msgs(thread, 0)
> EOF
>
> test_begin_subtest "recursive traversal of replies (no crash)"
> -test_subtest_known_broken
> test_python < recurse.py
> error=$?
> test_expect_equal "${error}" 0
>
> test_begin_subtest "recursive traversal of replies (output)"
> -test_subtest_known_broken
> test_python < recurse.py
> tail -n 10 < OUTPUT > OUTPUT.sample
> cat < EXPECTED
oh, nice debugging! And yes, this seems like the right fix, glad the
mechanism was at least already in place.
With respect to the docs, they seem clear enough. Probably I missed
this or I simply didn't realise that the replies iter is basically a
thread owning it.
Only thing I'd do different is write a pytest test for this as well, but
than I wouldn't have written the notmuch tests. So I don't think this
matters that much.
Cheers,
Floris
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
[PATCH 2/2] python-cffi: returned OwnedMessage objects from Message.replies
If we return regular Message objects, python will try to destroy them,
and the underlying notmuch object, causing e.g. the crash [1].
[1]: id:87sfu6utxg@tethera.net
---
bindings/python-cffi/notmuch2/_message.py | 4 ++--
test/T392-python-cffi-notmuch.sh | 2 --
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/bindings/python-cffi/notmuch2/_message.py
b/bindings/python-cffi/notmuch2/_message.py
index a460d8c1..aa1cb875 100644
--- a/bindings/python-cffi/notmuch2/_message.py
+++ b/bindings/python-cffi/notmuch2/_message.py
@@ -371,14 +371,14 @@ class Message(base.NotmuchObject):
This method will only work if the message was created from a
thread. Otherwise it will yield no results.
-:returns: An iterator yielding :class:`Message` instances.
+:returns: An iterator yielding :class:`OwnedMessage` instances.
:rtype: MessageIter
"""
# The notmuch_messages_valid call accepts NULL and this will
# become an empty iterator, raising StopIteration immediately.
# Hence no return value checking here.
msgs_p = capi.lib.notmuch_message_get_replies(self._msg_p)
-return MessageIter(self, msgs_p, db=self._db)
+return MessageIter(self, msgs_p, db=self._db, msg_cls=OwnedMessage)
def __hash__(self):
return hash(self.messageid)
diff --git a/test/T392-python-cffi-notmuch.sh b/test/T392-python-cffi-notmuch.sh
index 50012c55..15c8fc6b 100755
--- a/test/T392-python-cffi-notmuch.sh
+++ b/test/T392-python-cffi-notmuch.sh
@@ -24,13 +24,11 @@ show_msgs(thread, 0)
EOF
test_begin_subtest "recursive traversal of replies (no crash)"
-test_subtest_known_broken
test_python < recurse.py
error=$?
test_expect_equal "${error}" 0
test_begin_subtest "recursive traversal of replies (output)"
-test_subtest_known_broken
test_python < recurse.py
tail -n 10 < OUTPUT > OUTPUT.sample
cat < EXPECTED
--
2.34.1
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
