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

mininode tests broken with Python 3.4.2 (Debian Jessie) #13745

Closed
domob1812 opened this issue Jul 23, 2018 · 4 comments · Fixed by #13747
Closed

mininode tests broken with Python 3.4.2 (Debian Jessie) #13745

domob1812 opened this issue Jul 23, 2018 · 4 comments · Fixed by #13747
Labels
Tests

Comments

@domob1812
Copy link
Contributor

The recent change in #13715 broke mininode tests on systems with Python 3.4.2 (as included in Debian Jessie aka oldstable). This is because the method is_closing is only introduced in Python 3.4.4.

A workaround would be to add a hasattr condition that only checks is_closing if it is actually available - keeping the benefits of #13715 for newer systems while not breaking old systems (basically falling back to the behaviour before #13715 for them). If that's a desired fix, I'm happy to create a PR for it.
@domob1812 domob1812 mentioned this issue Jul 23, 2018
Merged
@fanquake fanquake added the Tests label Jul 23, 2018
domob1812 added a commit to domob1812/namecoin-core that referenced this issue Jul 23, 2018
@domob1812
Merge branch 'bitcoin' into auxpow
b233b9c 
bitcoin/bitcoin#13715 broke compatibility
with Debian Jessie.  For now, this includes a workaround for this,
although we might remove that later and drop compatibility ourselves.
See also bitcoin/bitcoin#13745.

Note that the new rpc_psbt.py test is broken upstream, see
bitcoin/bitcoin#13738.

Conflicts:
	src/rpc/rawtransaction.h
	src/wallet/rpcwallet.cpp
@maflcko
Copy link
Member

maflcko commented Jul 23, 2018

I am happy with that fallback, if the tests would otherwise be impossible to run. Just noting that you probably shouldn't run anything older than python 3.4.8 and that python 3.4 is completely EOL Q1 2019: https://devguide.python.org/#status-of-python-branches

@marcoagner
Copy link
Contributor

marcoagner commented Jul 23, 2018 •
edited

Sorry for the trouble.
I think a simple and explicit if condition in send_message would be fine so it stays obvious and simple for when Python <=3.4.2 is dropped:

if hasattr(self._transport, "is_closing"):                              
    NetworkThread.network_event_loop.call_soon_threadsafe(lambda: self._transport and not self._transport.is_closing() and self._transport.write(tmsg))
else:
    # is_closing not implemented before Python 3.4.4                                                             
    NetworkThread.network_event_loop.call_soon_threadsafe(lambda: self._transport and self._transport.write(tmsg))

*Verbose/redundant but very obvious.

Unless #13579 should be solved for Python <3.4.4 too; then another workaround should be implemented.
LMK if you are doing it or would like me to do it.

@domob1812
Copy link
Contributor Author

Sounds good to me. I don't know if #13579 should be solved also for old systems, but will prepare a PR with the simple workaround for now.

@domob1812 domob1812 mentioned this issue Jul 23, 2018
Merged
@domob1812
Copy link
Contributor Author

Proposed fix is in #13747, which simply disables the new is_closing check for systems that don't have it (as discussed above).

domob1812 added a commit to domob1812/bitcoin that referenced this issue Jul 23, 2018
@domob1812
Skip is_closing() check when not available.
64b9f27 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
maflcko pushed a commit that referenced this issue Jul 23, 2018
Merge #13747: tests: Skip P2PConnection's is_closing() check when not…
ab28b5b 
… available

64b9f27 Skip is_closing() check when not available. (Daniel Kraft)

Pull request description:

  #13715 introduced a new check for `_transport.is_closing()` in mininode's `P2PConnection`'s.  This function is [only available from Python 3.4.4](https://docs.python.org/3.4/library/asyncio-protocol.html#asyncio.BaseTransport.is_closing), though, while Bitcoin Core is supposed to support all Python 3.4 versions.

  In this change, we make the check conditional on `is_closing` being available.  If it is not, then we revert to the behaviour before the check was introduced; this means that #13579 is not fixed for old systems, but at least the tests work as they used to do before.

  This includes a small refactoring from a one-line lambda to an inline function, because this makes the code easier to read with more and more conditions being added.

  Fixes #13745.

Tree-SHA512: 15be03b8b49c40a946c5b354c5974858d14dc46283ad48ee25d9e269377077ce741c6b379b3f6581ab981cb65be799809afbb99da278caaa2d8d870fa4fb748f
Bushstar pushed a commit to Bushstar/Feathercoin that referenced this issue Jul 24, 2018
@domob1812 @Bushstar
Skip is_closing() check when not available.
4cf7d34 
bitcoin/bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin/bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin/bitcoin#13745.
HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this issue Sep 12, 2018
@domob1812
Skip is_closing() check when not available.
abcd4e4 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
This was referenced Sep 12, 2018
Merged
Merged
jfhk pushed a commit to jfhk/bitcoin that referenced this issue Nov 14, 2018
@domob1812 @jfhk
Skip is_closing() check when not available.
b8a0a20 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
furszy pushed a commit to furszy/bitcoin-core that referenced this issue Jun 4, 2021
@domob1812 @furszy
Skip is_closing() check when not available.
4331a67 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
furszy pushed a commit to furszy/bitcoin-core that referenced this issue Jun 9, 2021
@domob1812 @furszy
Skip is_closing() check when not available.
1460e5b 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
furszy pushed a commit to furszy/bitcoin-core that referenced this issue Jun 10, 2021
@domob1812 @furszy
Skip is_closing() check when not available.
c9f265c 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
furszy pushed a commit to furszy/bitcoin-core that referenced this issue Jun 12, 2021
@domob1812 @furszy
Skip is_closing() check when not available.
7038ae0 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
furszy pushed a commit to furszy/bitcoin-core that referenced this issue Jun 12, 2021
@domob1812 @furszy
Skip is_closing() check when not available.
649cc97 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
furszy pushed a commit to furszy/bitcoin-core that referenced this issue Jun 14, 2021
@domob1812 @furszy
Skip is_closing() check when not available.
37aa223 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
furszy pushed a commit to furszy/bitcoin-core that referenced this issue Jun 22, 2021
@domob1812 @furszy
Skip is_closing() check when not available.
6ca86b3 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
furszy pushed a commit to furszy/bitcoin-core that referenced this issue Jun 22, 2021
@domob1812 @furszy
Skip is_closing() check when not available.
77b9a43 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
furszy pushed a commit to furszy/bitcoin-core that referenced this issue Jun 24, 2021
@domob1812 @furszy
Skip is_closing() check when not available.
0433f23 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
furszy pushed a commit to furszy/bitcoin-core that referenced this issue Jun 28, 2021
@domob1812 @furszy
Skip is_closing() check when not available.
db28a53 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
furszy pushed a commit to furszy/bitcoin-core that referenced this issue Jun 28, 2021
@domob1812 @furszy
Skip is_closing() check when not available.
2f49b8e 
bitcoin#13715 introduced a new check
for _transport.is_closing() in mininode's P2PConnection's.  This function
is only available from Python 3.4.4, though, while Bitcoin is supposed
to support all Python 3.4 versions.

In this change, we make the check conditional on is_closing() being
available.  If it is not, then we revert to the behaviour before the
check was introduced; this means that
bitcoin#13579 is not fixed for old
systems, but at least the tests work as they used to do before.

This includes a small refactoring from a one-line lambda to an
inline function, because this makes the code easier to read with more
and more conditions being added.

Fixes bitcoin#13745.
UdjinM6 pushed a commit to UdjinM6/dash that referenced this issue Jun 29, 2021
@UdjinM6
Merge bitcoin#13747: tests: Skip P2PConnection's is_closing() check w…
cf0d699 
…hen not available

64b9f27 Skip is_closing() check when not available. (Daniel Kraft)

Pull request description:

  bitcoin#13715 introduced a new check for `_transport.is_closing()` in mininode's `P2PConnection`'s.  This function is [only available from Python 3.4.4](https://docs.python.org/3.4/library/asyncio-protocol.html#asyncio.BaseTransport.is_closing), though, while Bitcoin Core is supposed to support all Python 3.4 versions.

  In this change, we make the check conditional on `is_closing` being available.  If it is not, then we revert to the behaviour before the check was introduced; this means that bitcoin#13579 is not fixed for old systems, but at least the tests work as they used to do before.

  This includes a small refactoring from a one-line lambda to an inline function, because this makes the code easier to read with more and more conditions being added.

  Fixes bitcoin#13745.

Tree-SHA512: 15be03b8b49c40a946c5b354c5974858d14dc46283ad48ee25d9e269377077ce741c6b379b3f6581ab981cb65be799809afbb99da278caaa2d8d870fa4fb748f
UdjinM6 pushed a commit to UdjinM6/dash that referenced this issue Jun 29, 2021
@UdjinM6
Merge bitcoin#13747: tests: Skip P2PConnection's is_closing() check w…
010ff7d 
…hen not available

64b9f27 Skip is_closing() check when not available. (Daniel Kraft)

Pull request description:

  bitcoin#13715 introduced a new check for `_transport.is_closing()` in mininode's `P2PConnection`'s.  This function is [only available from Python 3.4.4](https://docs.python.org/3.4/library/asyncio-protocol.html#asyncio.BaseTransport.is_closing), though, while Bitcoin Core is supposed to support all Python 3.4 versions.

  In this change, we make the check conditional on `is_closing` being available.  If it is not, then we revert to the behaviour before the check was introduced; this means that bitcoin#13579 is not fixed for old systems, but at least the tests work as they used to do before.

  This includes a small refactoring from a one-line lambda to an inline function, because this makes the code easier to read with more and more conditions being added.

  Fixes bitcoin#13745.

Tree-SHA512: 15be03b8b49c40a946c5b354c5974858d14dc46283ad48ee25d9e269377077ce741c6b379b3f6581ab981cb65be799809afbb99da278caaa2d8d870fa4fb748f
UdjinM6 pushed a commit to UdjinM6/dash that referenced this issue Jul 1, 2021
@UdjinM6
Merge bitcoin#13747: tests: Skip P2PConnection's is_closing() check w…
1395a6a 
…hen not available

64b9f27 Skip is_closing() check when not available. (Daniel Kraft)

Pull request description:

  bitcoin#13715 introduced a new check for `_transport.is_closing()` in mininode's `P2PConnection`'s.  This function is [only available from Python 3.4.4](https://docs.python.org/3.4/library/asyncio-protocol.html#asyncio.BaseTransport.is_closing), though, while Bitcoin Core is supposed to support all Python 3.4 versions.

  In this change, we make the check conditional on `is_closing` being available.  If it is not, then we revert to the behaviour before the check was introduced; this means that bitcoin#13579 is not fixed for old systems, but at least the tests work as they used to do before.

  This includes a small refactoring from a one-line lambda to an inline function, because this makes the code easier to read with more and more conditions being added.

  Fixes bitcoin#13745.

Tree-SHA512: 15be03b8b49c40a946c5b354c5974858d14dc46283ad48ee25d9e269377077ce741c6b379b3f6581ab981cb65be799809afbb99da278caaa2d8d870fa4fb748f
UdjinM6 pushed a commit to UdjinM6/dash that referenced this issue Jul 2, 2021
@UdjinM6
Merge bitcoin#13747: tests: Skip P2PConnection's is_closing() check w…
5f50a8c 
…hen not available

64b9f27 Skip is_closing() check when not available. (Daniel Kraft)

Pull request description:

  bitcoin#13715 introduced a new check for `_transport.is_closing()` in mininode's `P2PConnection`'s.  This function is [only available from Python 3.4.4](https://docs.python.org/3.4/library/asyncio-protocol.html#asyncio.BaseTransport.is_closing), though, while Bitcoin Core is supposed to support all Python 3.4 versions.

  In this change, we make the check conditional on `is_closing` being available.  If it is not, then we revert to the behaviour before the check was introduced; this means that bitcoin#13579 is not fixed for old systems, but at least the tests work as they used to do before.

  This includes a small refactoring from a one-line lambda to an inline function, because this makes the code easier to read with more and more conditions being added.

  Fixes bitcoin#13745.

Tree-SHA512: 15be03b8b49c40a946c5b354c5974858d14dc46283ad48ee25d9e269377077ce741c6b379b3f6581ab981cb65be799809afbb99da278caaa2d8d870fa4fb748f
UdjinM6 pushed a commit to UdjinM6/dash that referenced this issue Jul 2, 2021
@UdjinM6
Merge bitcoin#13747: tests: Skip P2PConnection's is_closing() check w…
ea58f30 
…hen not available

64b9f27 Skip is_closing() check when not available. (Daniel Kraft)

Pull request description:

  bitcoin#13715 introduced a new check for `_transport.is_closing()` in mininode's `P2PConnection`'s.  This function is [only available from Python 3.4.4](https://docs.python.org/3.4/library/asyncio-protocol.html#asyncio.BaseTransport.is_closing), though, while Bitcoin Core is supposed to support all Python 3.4 versions.

  In this change, we make the check conditional on `is_closing` being available.  If it is not, then we revert to the behaviour before the check was introduced; this means that bitcoin#13579 is not fixed for old systems, but at least the tests work as they used to do before.

  This includes a small refactoring from a one-line lambda to an inline function, because this makes the code easier to read with more and more conditions being added.

  Fixes bitcoin#13745.

Tree-SHA512: 15be03b8b49c40a946c5b354c5974858d14dc46283ad48ee25d9e269377077ce741c6b379b3f6581ab981cb65be799809afbb99da278caaa2d8d870fa4fb748f
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
gades pushed a commit to cosanta/cosanta-core that referenced this issue May 4, 2022
@gades
Merge bitcoin#13747: tests: Skip P2PConnection's is_closing() check w…
83977f6 
…hen not available

64b9f27 Skip is_closing() check when not available. (Daniel Kraft)

Pull request description:

  bitcoin#13715 introduced a new check for `_transport.is_closing()` in mininode's `P2PConnection`'s.  This function is [only available from Python 3.4.4](https://docs.python.org/3.4/library/asyncio-protocol.html#asyncio.BaseTransport.is_closing), though, while Bitcoin Core is supposed to support all Python 3.4 versions.

  In this change, we make the check conditional on `is_closing` being available.  If it is not, then we revert to the behaviour before the check was introduced; this means that bitcoin#13579 is not fixed for old systems, but at least the tests work as they used to do before.

  This includes a small refactoring from a one-line lambda to an inline function, because this makes the code easier to read with more and more conditions being added.

  Fixes bitcoin#13745.

Tree-SHA512: 15be03b8b49c40a946c5b354c5974858d14dc46283ad48ee25d9e269377077ce741c6b379b3f6581ab981cb65be799809afbb99da278caaa2d8d870fa4fb748f
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

tests: Skip P2PConnection's is_closing() check when not available domob1812/bitcoin
4 participants
@fanquake @domob1812 @marcoagner @maflcko