test: Remove python3.5 workaround #27378

1. 
   maflcko commented at 8:29 AM on March 31, 2023: member

   Remove workaround for a bug that is long fixed in a EOL python version, that isn't used by us.

   If the workaround is still needed, it should at least log the exception before silently discarding it, so that debugging is possible/easier.

2. maflcko force-pushed on Mar 31, 2023
3. 
   DrahtBot commented at 8:29 AM on March 31, 2023: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	fanquake

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. DrahtBot added the label Tests on Mar 31, 2023
5. maflcko force-pushed on Mar 31, 2023
6. 
   fanquake commented at 9:11 AM on March 31, 2023: member

   Concept ACK

7. test: Remove python3.5 workaround in authproxy

   Also, move the burden of checking for a timeout to the client and
   disable the timeout on the server. This should avoid intermittent issues
   in slow tests (for example mining_getblocktemplate_longpoll.py, or
   feature_dbcrash.py), or possibly when the server is running slow (for
   example in valgrind).  There shouldn't be any downside in tests caused
   by a high rpcservertimeout.

   fae66fceb3
8. in test/functional/test_framework/util.py:394 in ddddb17261 outdated

    389 | @@ -390,6 +390,8 @@ def write_config(config_path, *, n, chain, extra_config="", disable_autoconnect=
    390 |              f.write("[{}]\n".format(chain_name_conf_section))
    391 |          f.write("port=" + str(p2p_port(n)) + "\n")
    392 |          f.write("rpcport=" + str(rpc_port(n)) + "\n")
    393 | +        # Disable server-side timeouts to avoid intermittent issues
    394 | +        f.write("rpcservertimeout=99000\n")
   

   ---

   ---

   

   fanquake commented at 10:57 AM on March 31, 2023:

   Can drop -rpcservertimeout=900 from feature_dbcrash given we are setting the default here?

   ---

   

   maflcko commented at 12:31 PM on March 31, 2023:

   thx, done

9. maflcko force-pushed on Mar 31, 2023
10. fanquake requested review from theStack on Mar 31, 2023
11. 
    stickies-v commented at 3:37 PM on March 31, 2023: contributor

    This PR seems to remove 2 workarounds: https://bugs.python.org/issue3566 and https://bugs.python.org/issue33450. 3566 is marked as resolved (and indeed for Python 3.5) so I think that can be safely removed. 33450 however is still open and also affects Python 3.6. In the description, I can't find in which macOS version this stopped being an issue. Do you have any sources that confirm we can indeed remove this safely?

12. 
    fanquake commented at 3:54 PM on March 31, 2023: member

    however is still open and also affects Python 3.6.

    Our minimum required Python is 3.7.

    https://bugs.python.org/issue33450

    Looks like the new home for this issue is also https://github.com/python/cpython/issues/77631

13. 
    maflcko commented at 5:03 PM on March 31, 2023: member

    This patch here disables the server timeout, so there shouldn't be any reason for the socket to be closed while sending on macos. However, if that workaround is still needed, it should be guarded by a macos check. So my suggestion would be to assume the workaround is no longer needed, unless someone can prove otherwise?

14. 
    fanquake commented at 11:51 AM on April 2, 2023: member

    ACK fae66fceb3147385320593d1e15faf290b0f4caf

    I agree. Not seeing any issues with this on macOS, so lets merge, and revisit if any issues occur.

15. fanquake merged this on Apr 2, 2023
16. fanquake closed this on Apr 2, 2023

    ---

17. sidhujag referenced this in commit fe7bc5d961 on Apr 2, 2023
18. maflcko deleted the branch on Apr 4, 2023
19. kwvg referenced this in commit f4b02d1a9f on May 10, 2023
20. kwvg referenced this in commit 30a0374e47 on May 10, 2023
21. bitcoin locked this on Apr 3, 2024

Contributors

maflckoDrahtBotfanquakestickies-v

Review Requested
theStack