From cebf62176ee7ec064936c111aa28cb65f69d649b Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 10 Apr 2023 11:40:59 -0400 Subject: [PATCH 01/38] WIP add logging to decode_cbor. --- src/allmydata/storage/http_client.py | 46 +++++++++++++++------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index ea142ed85..131f23846 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -447,24 +447,28 @@ class StorageClient(object): method, url, headers=headers, timeout=timeout, **kwargs ) - def decode_cbor(self, response, schema: Schema): + async def decode_cbor(self, response, schema: Schema) -> object: """Given HTTP response, return decoded CBOR body.""" - - def got_content(f: BinaryIO): - data = f.read() - schema.validate_cbor(data) - return loads(data) - - if response.code > 199 and response.code < 300: - content_type = get_content_type(response.headers) - if content_type == CBOR_MIME_TYPE: - return limited_content(response, self._clock).addCallback(got_content) + with start_action(action_type="allmydata:storage:http-client:decode-cbor"): + if response.code > 199 and response.code < 300: + content_type = get_content_type(response.headers) + if content_type == CBOR_MIME_TYPE: + f = await limited_content(response, self._clock) + data = f.read() + schema.validate_cbor(data) + return loads(data) + else: + raise ClientException( + -1, + "Server didn't send CBOR, content type is {}".format( + content_type + ), + ) else: - raise ClientException(-1, "Server didn't send CBOR") - else: - return treq.content(response).addCallback( - lambda data: fail(ClientException(response.code, response.phrase, data)) - ) + data = ( + await limited_content(response, self._clock, max_length=10_000) + ).read() + raise ClientException(response.code, response.phrase, data) @define(hash=True) @@ -475,14 +479,14 @@ class StorageClientGeneral(object): _client: StorageClient - @inlineCallbacks - def get_version(self): + @async_to_deferred + async def get_version(self): """ Return the version metadata for the server. """ url = self._client.relative_url("/storage/v1/version") - response = yield self._client.request("GET", url) - decoded_response = yield self._client.decode_cbor( + response = await self._client.request("GET", url) + decoded_response = await self._client.decode_cbor( response, _SCHEMAS["get_version"] ) # Add some features we know are true because the HTTP API @@ -496,7 +500,7 @@ class StorageClientGeneral(object): b"prevents-read-past-end-of-share-data": True, } ) - returnValue(decoded_response) + return decoded_response @inlineCallbacks def add_or_renew_lease( From 2a7616e0bebe29865767141255e36e9058db77e6 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 12 Apr 2023 16:43:46 -0400 Subject: [PATCH 02/38] Get tests passing again. --- src/allmydata/storage/http_client.py | 36 ++++++++++++------------- src/allmydata/test/test_storage_http.py | 3 ++- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index 131f23846..7fc68c902 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -651,8 +651,8 @@ class StorageClientImmutables(object): _client: StorageClient - @inlineCallbacks - def create( + @async_to_deferred + async def create( self, storage_index, share_numbers, @@ -679,7 +679,7 @@ class StorageClientImmutables(object): ) message = {"share-numbers": share_numbers, "allocated-size": allocated_size} - response = yield self._client.request( + response = await self._client.request( "POST", url, lease_renew_secret=lease_renew_secret, @@ -687,14 +687,12 @@ class StorageClientImmutables(object): upload_secret=upload_secret, message_to_serialize=message, ) - decoded_response = yield self._client.decode_cbor( + decoded_response = await self._client.decode_cbor( response, _SCHEMAS["allocate_buckets"] ) - returnValue( - ImmutableCreateResult( - already_have=decoded_response["already-have"], - allocated=decoded_response["allocated"], - ) + return ImmutableCreateResult( + already_have=decoded_response["already-have"], + allocated=decoded_response["allocated"], ) @inlineCallbacks @@ -720,8 +718,8 @@ class StorageClientImmutables(object): response.code, ) - @inlineCallbacks - def write_share_chunk( + @async_to_deferred + async def write_share_chunk( self, storage_index, share_number, upload_secret, offset, data ): # type: (bytes, int, bytes, int, bytes) -> Deferred[UploadProgress] """ @@ -741,7 +739,7 @@ class StorageClientImmutables(object): _encode_si(storage_index), share_number ) ) - response = yield self._client.request( + response = await self._client.request( "PATCH", url, upload_secret=upload_secret, @@ -765,13 +763,13 @@ class StorageClientImmutables(object): raise ClientException( response.code, ) - body = yield self._client.decode_cbor( + body = await self._client.decode_cbor( response, _SCHEMAS["immutable_write_share_chunk"] ) remaining = RangeMap() for chunk in body["required"]: remaining.set(True, chunk["begin"], chunk["end"]) - returnValue(UploadProgress(finished=finished, required=remaining)) + return UploadProgress(finished=finished, required=remaining) def read_share_chunk( self, storage_index, share_number, offset, length @@ -783,21 +781,21 @@ class StorageClientImmutables(object): self._client, "immutable", storage_index, share_number, offset, length ) - @inlineCallbacks - def list_shares(self, storage_index: bytes) -> Deferred[set[int]]: + @async_to_deferred + async def list_shares(self, storage_index: bytes) -> Deferred[set[int]]: """ Return the set of shares for a given storage index. """ url = self._client.relative_url( "/storage/v1/immutable/{}/shares".format(_encode_si(storage_index)) ) - response = yield self._client.request( + response = await self._client.request( "GET", url, ) if response.code == http.OK: - body = yield self._client.decode_cbor(response, _SCHEMAS["list_shares"]) - returnValue(set(body)) + body = await self._client.decode_cbor(response, _SCHEMAS["list_shares"]) + return set(body) else: raise ClientException(response.code) diff --git a/src/allmydata/test/test_storage_http.py b/src/allmydata/test/test_storage_http.py index ea93ad360..eca2be1c1 100644 --- a/src/allmydata/test/test_storage_http.py +++ b/src/allmydata/test/test_storage_http.py @@ -34,7 +34,7 @@ from hyperlink import DecodedURL from collections_extended import RangeMap from twisted.internet.task import Clock, Cooperator from twisted.internet.interfaces import IReactorTime, IReactorFromThreads -from twisted.internet.defer import CancelledError, Deferred +from twisted.internet.defer import CancelledError, Deferred, ensureDeferred from twisted.web import http from twisted.web.http_headers import Headers from werkzeug import routing @@ -520,6 +520,7 @@ class HttpTestFixture(Fixture): Like ``result_of``, but supports fake reactor and ``treq`` testing infrastructure necessary to support asynchronous HTTP server endpoints. """ + d = ensureDeferred(d) result = [] error = [] d.addCallbacks(result.append, error.append) From 3997eaaf9048af3daca9cc291875f17b2a403218 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 12 Apr 2023 17:00:31 -0400 Subject: [PATCH 03/38] Fix type annotations. --- src/allmydata/storage/http_client.py | 49 ++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index 7fc68c902..b1877cd5f 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -5,7 +5,7 @@ HTTP client that talks to the HTTP storage server. from __future__ import annotations from eliot import start_action, register_exception_extractor -from typing import Union, Optional, Sequence, Mapping, BinaryIO +from typing import Union, Optional, Sequence, Mapping, BinaryIO, cast, TypedDict from base64 import b64encode from io import BytesIO from os import SEEK_END @@ -486,13 +486,17 @@ class StorageClientGeneral(object): """ url = self._client.relative_url("/storage/v1/version") response = await self._client.request("GET", url) - decoded_response = await self._client.decode_cbor( - response, _SCHEMAS["get_version"] + decoded_response = cast( + dict[bytes, object], + await self._client.decode_cbor(response, _SCHEMAS["get_version"]), ) # Add some features we know are true because the HTTP API # specification requires them and because other parts of the storage # client implementation assumes they will be present. - decoded_response[b"http://allmydata.org/tahoe/protocols/storage/v1"].update( + cast( + dict[bytes, object], + decoded_response[b"http://allmydata.org/tahoe/protocols/storage/v1"], + ).update( { b"tolerates-immutable-read-overrun": True, b"delete-mutable-shares-with-zero-length-writev": True, @@ -687,8 +691,9 @@ class StorageClientImmutables(object): upload_secret=upload_secret, message_to_serialize=message, ) - decoded_response = await self._client.decode_cbor( - response, _SCHEMAS["allocate_buckets"] + decoded_response = cast( + dict[str, set[int]], + await self._client.decode_cbor(response, _SCHEMAS["allocate_buckets"]), ) return ImmutableCreateResult( already_have=decoded_response["already-have"], @@ -763,8 +768,11 @@ class StorageClientImmutables(object): raise ClientException( response.code, ) - body = await self._client.decode_cbor( - response, _SCHEMAS["immutable_write_share_chunk"] + body = cast( + dict[str, list[dict[str, int]]], + await self._client.decode_cbor( + response, _SCHEMAS["immutable_write_share_chunk"] + ), ) remaining = RangeMap() for chunk in body["required"]: @@ -794,7 +802,10 @@ class StorageClientImmutables(object): url, ) if response.code == http.OK: - body = await self._client.decode_cbor(response, _SCHEMAS["list_shares"]) + body = cast( + set[int], + await self._client.decode_cbor(response, _SCHEMAS["list_shares"]), + ) return set(body) else: raise ClientException(response.code) @@ -865,6 +876,12 @@ class ReadTestWriteResult: reads: Mapping[int, Sequence[bytes]] +# Result type for mutable read/test/write HTTP response. +MUTABLE_RTW = TypedDict( + "MUTABLE_RTW", {"success": bool, "data": dict[int, list[bytes]]} +) + + @frozen class StorageClientMutables: """ @@ -911,8 +928,11 @@ class StorageClientMutables: message_to_serialize=message, ) if response.code == http.OK: - result = await self._client.decode_cbor( - response, _SCHEMAS["mutable_read_test_write"] + result = cast( + MUTABLE_RTW, + await self._client.decode_cbor( + response, _SCHEMAS["mutable_read_test_write"] + ), ) return ReadTestWriteResult(success=result["success"], reads=result["data"]) else: @@ -942,8 +962,11 @@ class StorageClientMutables: ) response = await self._client.request("GET", url) if response.code == http.OK: - return await self._client.decode_cbor( - response, _SCHEMAS["mutable_list_shares"] + return cast( + set[int], + await self._client.decode_cbor( + response, _SCHEMAS["mutable_list_shares"] + ), ) else: raise ClientException(response.code) From 8bda370b30ef187d321840ecea97c56a439e1280 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 12 Apr 2023 17:00:47 -0400 Subject: [PATCH 04/38] News fragment. --- newsfragments/4005.misc | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/4005.misc diff --git a/newsfragments/4005.misc b/newsfragments/4005.misc new file mode 100644 index 000000000..e69de29bb From 840ed0bf47561c7c9720c91deab3d5a5c56a7b35 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 12 Apr 2023 17:04:00 -0400 Subject: [PATCH 05/38] Unused imports. --- src/allmydata/storage/http_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index b1877cd5f..ef4005414 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -20,7 +20,7 @@ from werkzeug.datastructures import Range, ContentRange from twisted.web.http_headers import Headers from twisted.web import http from twisted.web.iweb import IPolicyForHTTPS, IResponse -from twisted.internet.defer import inlineCallbacks, returnValue, fail, Deferred, succeed +from twisted.internet.defer import inlineCallbacks, Deferred, succeed from twisted.internet.interfaces import ( IOpenSSLClientConnectionCreator, IReactorTime, From 33ab0ce0422ff6211bd614bb8429fbe2084b9e02 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 12 Apr 2023 17:10:33 -0400 Subject: [PATCH 06/38] Fix name. --- newsfragments/{4005.misc => 4005.minor} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename newsfragments/{4005.misc => 4005.minor} (100%) diff --git a/newsfragments/4005.misc b/newsfragments/4005.minor similarity index 100% rename from newsfragments/4005.misc rename to newsfragments/4005.minor From af845a40c6dd1fe626771c63dff0c8dd7a6d86c8 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 13 Apr 2023 09:38:33 -0400 Subject: [PATCH 07/38] Fix type annotations, removing Deferred in particular. --- src/allmydata/storage/http_client.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index ef4005414..e21cfc5cc 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -658,13 +658,13 @@ class StorageClientImmutables(object): @async_to_deferred async def create( self, - storage_index, - share_numbers, - allocated_size, - upload_secret, - lease_renew_secret, - lease_cancel_secret, - ): # type: (bytes, set[int], int, bytes, bytes, bytes) -> Deferred[ImmutableCreateResult] + storage_index: bytes, + share_numbers: set[int], + allocated_size: int, + upload_secret: bytes, + lease_renew_secret: bytes, + lease_cancel_secret: bytes, + ) -> ImmutableCreateResult: """ Create a new storage index for an immutable. @@ -725,8 +725,13 @@ class StorageClientImmutables(object): @async_to_deferred async def write_share_chunk( - self, storage_index, share_number, upload_secret, offset, data - ): # type: (bytes, int, bytes, int, bytes) -> Deferred[UploadProgress] + self, + storage_index: bytes, + share_number: int, + upload_secret: bytes, + offset: int, + data: bytes, + ) -> UploadProgress: """ Upload a chunk of data for a specific share. @@ -790,7 +795,7 @@ class StorageClientImmutables(object): ) @async_to_deferred - async def list_shares(self, storage_index: bytes) -> Deferred[set[int]]: + async def list_shares(self, storage_index: bytes) -> set[int]: """ Return the set of shares for a given storage index. """ From 464b47619028e9d194e660ad461467acaf8986b4 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 13 Apr 2023 13:11:17 -0400 Subject: [PATCH 08/38] Work on 3.8. --- src/allmydata/storage/http_client.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index e21cfc5cc..fc165bedd 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -5,7 +5,7 @@ HTTP client that talks to the HTTP storage server. from __future__ import annotations from eliot import start_action, register_exception_extractor -from typing import Union, Optional, Sequence, Mapping, BinaryIO, cast, TypedDict +from typing import Union, Optional, Sequence, Mapping, BinaryIO, cast, TypedDict, Set from base64 import b64encode from io import BytesIO from os import SEEK_END @@ -487,14 +487,14 @@ class StorageClientGeneral(object): url = self._client.relative_url("/storage/v1/version") response = await self._client.request("GET", url) decoded_response = cast( - dict[bytes, object], + Mapping[bytes, object], await self._client.decode_cbor(response, _SCHEMAS["get_version"]), ) # Add some features we know are true because the HTTP API # specification requires them and because other parts of the storage # client implementation assumes they will be present. cast( - dict[bytes, object], + Mapping[bytes, object], decoded_response[b"http://allmydata.org/tahoe/protocols/storage/v1"], ).update( { @@ -692,7 +692,7 @@ class StorageClientImmutables(object): message_to_serialize=message, ) decoded_response = cast( - dict[str, set[int]], + Mapping[str, Set[int]], await self._client.decode_cbor(response, _SCHEMAS["allocate_buckets"]), ) return ImmutableCreateResult( @@ -774,7 +774,7 @@ class StorageClientImmutables(object): response.code, ) body = cast( - dict[str, list[dict[str, int]]], + Mapping[str, Sequence[Mapping[str, int]]], await self._client.decode_cbor( response, _SCHEMAS["immutable_write_share_chunk"] ), @@ -795,7 +795,7 @@ class StorageClientImmutables(object): ) @async_to_deferred - async def list_shares(self, storage_index: bytes) -> set[int]: + async def list_shares(self, storage_index: bytes) -> Set[int]: """ Return the set of shares for a given storage index. """ @@ -808,7 +808,7 @@ class StorageClientImmutables(object): ) if response.code == http.OK: body = cast( - set[int], + Set[int], await self._client.decode_cbor(response, _SCHEMAS["list_shares"]), ) return set(body) @@ -881,9 +881,10 @@ class ReadTestWriteResult: reads: Mapping[int, Sequence[bytes]] -# Result type for mutable read/test/write HTTP response. +# Result type for mutable read/test/write HTTP response. Can't just use +# dict[int,list[bytes]] because on Python 3.8 that will error out. MUTABLE_RTW = TypedDict( - "MUTABLE_RTW", {"success": bool, "data": dict[int, list[bytes]]} + "MUTABLE_RTW", {"success": bool, "data": Mapping[int, Sequence[bytes]]} ) @@ -958,7 +959,7 @@ class StorageClientMutables: ) @async_to_deferred - async def list_shares(self, storage_index: bytes) -> set[int]: + async def list_shares(self, storage_index: bytes) -> Set[int]: """ List the share numbers for a given storage index. """ @@ -968,7 +969,7 @@ class StorageClientMutables: response = await self._client.request("GET", url) if response.code == http.OK: return cast( - set[int], + Set[int], await self._client.decode_cbor( response, _SCHEMAS["mutable_list_shares"] ), From 5dcbc00989c94e314a018a8a9d79027796e95ffe Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 10:18:55 -0400 Subject: [PATCH 09/38] News fragment. --- newsfragments/4012.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/4012.bugfix diff --git a/newsfragments/4012.bugfix b/newsfragments/4012.bugfix new file mode 100644 index 000000000..97dfe6aad --- /dev/null +++ b/newsfragments/4012.bugfix @@ -0,0 +1 @@ +The command-line tools now have a 60-second timeout on individual network reads/writes/connects; previously they could block forever in some situations. \ No newline at end of file From d7ee1637dfabc3654ea6be735fbcd2094d39c1f3 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 10:22:06 -0400 Subject: [PATCH 10/38] Set a timeout. --- src/allmydata/scripts/common_http.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 95099a2eb..7542a045f 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -62,9 +62,9 @@ def do_http(method, url, body=b""): assert body.read scheme, host, port, path = parse_url(url) if scheme == "http": - c = http_client.HTTPConnection(host, port) + c = http_client.HTTPConnection(host, port, timeout=60) elif scheme == "https": - c = http_client.HTTPSConnection(host, port) + c = http_client.HTTPSConnection(host, port, timeout=60) else: raise ValueError("unknown scheme '%s', need http or https" % scheme) c.putrequest(method, path) From 67702572a9d4ce03c6614207234ae909672d6df5 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 10:22:14 -0400 Subject: [PATCH 11/38] Do a little modernization. --- src/allmydata/scripts/common_http.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 7542a045f..4da1345c9 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -1,18 +1,11 @@ """ Ported to Python 3. """ -from __future__ import unicode_literals -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 import os from io import BytesIO -from six.moves import urllib, http_client +from http import client as http_client +import urllib import six import allmydata # for __full_version__ From 1823dd4c03b3d715ef896453542d0ac10e7f4aad Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 10:24:00 -0400 Subject: [PATCH 12/38] Switch to a slightly larger block size. --- src/allmydata/scripts/common_http.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 4da1345c9..4c0319d3c 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -55,9 +55,9 @@ def do_http(method, url, body=b""): assert body.read scheme, host, port, path = parse_url(url) if scheme == "http": - c = http_client.HTTPConnection(host, port, timeout=60) + c = http_client.HTTPConnection(host, port, timeout=60, blocksize=65536) elif scheme == "https": - c = http_client.HTTPSConnection(host, port, timeout=60) + c = http_client.HTTPSConnection(host, port, timeout=60, blocksize=65536) else: raise ValueError("unknown scheme '%s', need http or https" % scheme) c.putrequest(method, path) @@ -78,7 +78,7 @@ def do_http(method, url, body=b""): return BadResponse(url, err) while True: - data = body.read(8192) + data = body.read(65536) if not data: break c.send(data) From 2916984114bdb9ef85df5a34aa439f45c5a14cab Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 10:29:25 -0400 Subject: [PATCH 13/38] More modernization. --- src/allmydata/scripts/common_http.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 4c0319d3c..7d627a7ad 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -1,12 +1,11 @@ """ -Ported to Python 3. +Blocking HTTP client APIs. """ import os from io import BytesIO from http import client as http_client import urllib -import six import allmydata # for __full_version__ from allmydata.util.encodingutil import quote_output @@ -44,7 +43,7 @@ class BadResponse(object): def do_http(method, url, body=b""): if isinstance(body, bytes): body = BytesIO(body) - elif isinstance(body, six.text_type): + elif isinstance(body, str): raise TypeError("do_http body must be a bytestring, not unicode") else: # We must give a Content-Length header to twisted.web, otherwise it @@ -87,16 +86,14 @@ def do_http(method, url, body=b""): def format_http_success(resp): - # ensure_text() shouldn't be necessary when Python 2 is dropped. return quote_output( - "%s %s" % (resp.status, six.ensure_text(resp.reason)), + "%s %s" % (resp.status, resp.reason), quotemarks=False) def format_http_error(msg, resp): - # ensure_text() shouldn't be necessary when Python 2 is dropped. return quote_output( - "%s: %s %s\n%s" % (msg, resp.status, six.ensure_text(resp.reason), - six.ensure_text(resp.read())), + "%s: %s %s\n%s" % (msg, resp.status, resp.reason, + resp.read()), quotemarks=False) def check_http_error(resp, stderr): From 2d81ddc297b336607bc8eac691913e7e3ac2f178 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 11:15:47 -0400 Subject: [PATCH 14/38] Don't call str() on bytes. --- src/allmydata/scripts/common_http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 7d627a7ad..a2cae5a85 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -92,7 +92,7 @@ def format_http_success(resp): def format_http_error(msg, resp): return quote_output( - "%s: %s %s\n%s" % (msg, resp.status, resp.reason, + "%s: %s %s\n%r" % (msg, resp.status, resp.reason, resp.read()), quotemarks=False) From 1371ffe9dc7e6a6b0346daad1603f6414bbd1fc7 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 25 Apr 2023 08:14:26 -0400 Subject: [PATCH 15/38] Just have ruff in one place. --- setup.py | 5 ----- tox.ini | 4 +++- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/setup.py b/setup.py index 3358aa6c6..2418c6dbe 100644 --- a/setup.py +++ b/setup.py @@ -399,11 +399,6 @@ setup(name="tahoe-lafs", # also set in __init__.py "gpg", ], "test": [ - # Pin a specific pyflakes so we don't have different folks - # disagreeing on what is or is not a lint issue. We can bump - # this version from time to time, but we will do it - # intentionally. - "ruff==0.0.261", "coverage ~= 5.0", "mock", "tox ~= 3.0", diff --git a/tox.ini b/tox.ini index 99487bc83..5f18b6b95 100644 --- a/tox.ini +++ b/tox.ini @@ -101,7 +101,9 @@ commands = basepython = python3 skip_install = true deps = - ruff + # Pin a specific version so we get consistent outcomes; update this + # occasionally: + ruff == 0.0.263 towncrier # On macOS, git inside of towncrier needs $HOME. passenv = HOME From ebed5100b9cf2a208875eb2977da23364559881d Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 25 Apr 2023 08:16:12 -0400 Subject: [PATCH 16/38] Switch to longer timeout so it's unlikely to impact users. --- newsfragments/4012.bugfix | 2 +- src/allmydata/scripts/common_http.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/newsfragments/4012.bugfix b/newsfragments/4012.bugfix index 97dfe6aad..24d5bb49a 100644 --- a/newsfragments/4012.bugfix +++ b/newsfragments/4012.bugfix @@ -1 +1 @@ -The command-line tools now have a 60-second timeout on individual network reads/writes/connects; previously they could block forever in some situations. \ No newline at end of file +The command-line tools now have a 300-second timeout on individual network reads/writes/connects; previously they could block forever in some situations. \ No newline at end of file diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index a2cae5a85..46676b3f5 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -54,9 +54,9 @@ def do_http(method, url, body=b""): assert body.read scheme, host, port, path = parse_url(url) if scheme == "http": - c = http_client.HTTPConnection(host, port, timeout=60, blocksize=65536) + c = http_client.HTTPConnection(host, port, timeout=300, blocksize=65536) elif scheme == "https": - c = http_client.HTTPSConnection(host, port, timeout=60, blocksize=65536) + c = http_client.HTTPSConnection(host, port, timeout=300, blocksize=65536) else: raise ValueError("unknown scheme '%s', need http or https" % scheme) c.putrequest(method, path) From 558e3bf79785fef5a8c3525f323590b9c7c15e36 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 25 Apr 2023 08:46:57 -0400 Subject: [PATCH 17/38] Fix unnecessary conversion. --- src/allmydata/storage/http_client.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index fc165bedd..f786b8f30 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -807,11 +807,10 @@ class StorageClientImmutables(object): url, ) if response.code == http.OK: - body = cast( + return cast( Set[int], await self._client.decode_cbor(response, _SCHEMAS["list_shares"]), ) - return set(body) else: raise ClientException(response.code) From f9a1eedaeadb818315bef8158902a47c863bbc65 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 25 Apr 2023 12:31:37 -0400 Subject: [PATCH 18/38] Make timeout optional, enable it only for integration tests. --- integration/conftest.py | 6 ++++++ newsfragments/4012.bugfix | 1 - newsfragments/4012.minor | 0 src/allmydata/scripts/common_http.py | 11 +++++++++-- 4 files changed, 15 insertions(+), 3 deletions(-) delete mode 100644 newsfragments/4012.bugfix create mode 100644 newsfragments/4012.minor diff --git a/integration/conftest.py b/integration/conftest.py index 879649588..d76b2a9c7 100644 --- a/integration/conftest.py +++ b/integration/conftest.py @@ -4,6 +4,7 @@ Ported to Python 3. from __future__ import annotations +import os import sys import shutil from time import sleep @@ -49,6 +50,11 @@ from .util import ( ) +# No reason for HTTP requests to take longer than two minutes in the +# integration tests. See allmydata/scripts/common_http.py for usage. +os.environ["__TAHOE_CLI_HTTP_TIMEOUT"] = "120" + + # pytest customization hooks def pytest_addoption(parser): diff --git a/newsfragments/4012.bugfix b/newsfragments/4012.bugfix deleted file mode 100644 index 24d5bb49a..000000000 --- a/newsfragments/4012.bugfix +++ /dev/null @@ -1 +0,0 @@ -The command-line tools now have a 300-second timeout on individual network reads/writes/connects; previously they could block forever in some situations. \ No newline at end of file diff --git a/newsfragments/4012.minor b/newsfragments/4012.minor new file mode 100644 index 000000000..e69de29bb diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 46676b3f5..f138b9c07 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -53,10 +53,17 @@ def do_http(method, url, body=b""): assert body.seek assert body.read scheme, host, port, path = parse_url(url) + + # For testing purposes, allow setting a timeout on HTTP requests. If this + # ever become a user-facing feature, this should probably be a CLI option? + timeout = os.environ.get("__TAHOE_CLI_HTTP_TIMEOUT", None) + if timeout is not None: + timeout = float(timeout) + if scheme == "http": - c = http_client.HTTPConnection(host, port, timeout=300, blocksize=65536) + c = http_client.HTTPConnection(host, port, timeout=timeout, blocksize=65536) elif scheme == "https": - c = http_client.HTTPSConnection(host, port, timeout=300, blocksize=65536) + c = http_client.HTTPSConnection(host, port, timeout=timeout, blocksize=65536) else: raise ValueError("unknown scheme '%s', need http or https" % scheme) c.putrequest(method, path) From 3d0c872f4c3751d48c5cf1f2392ec431b11235f8 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 27 Apr 2023 10:44:10 -0400 Subject: [PATCH 19/38] restrict CI jobs to the wheelhouse --- .circleci/setup-virtualenv.sh | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.circleci/setup-virtualenv.sh b/.circleci/setup-virtualenv.sh index feccbbf23..7087c5120 100755 --- a/.circleci/setup-virtualenv.sh +++ b/.circleci/setup-virtualenv.sh @@ -26,12 +26,7 @@ shift || : # Tell pip where it can find any existing wheels. export PIP_FIND_LINKS="file://${WHEELHOUSE_PATH}" - -# It is tempting to also set PIP_NO_INDEX=1 but (a) that will cause problems -# between the time dependencies change and the images are re-built and (b) the -# upcoming-deprecations job wants to install some dependencies from github and -# it's awkward to get that done any earlier than the tox run. So, we don't -# set it. +export PIP_NO_INDEX="1" # Get everything else installed in it, too. "${BOOTSTRAP_VENV}"/bin/tox \ From f9269158baf5103e014bbd439944690f3138c1af Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 27 Apr 2023 10:46:58 -0400 Subject: [PATCH 20/38] news fragment --- newsfragments/4019.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/4019.minor diff --git a/newsfragments/4019.minor b/newsfragments/4019.minor new file mode 100644 index 000000000..e69de29bb From 4d5b9f2d0c88e411b0cb032a46b8b681d984703c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 27 Apr 2023 10:48:46 -0400 Subject: [PATCH 21/38] match the version in the docker image it is maybe wrong that we pin a specific version here and also only include a specific version (probably some interpretation of "the most recent release") in the docker image... --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 5f18b6b95..6e56496d4 100644 --- a/tox.ini +++ b/tox.ini @@ -36,7 +36,7 @@ deps = # happening at the time. The versions selected here are just the current # versions at the time. Bumping them to keep up with future releases is # fine as long as those releases are known to actually work. - pip==22.0.3 + pip==22.3.1 setuptools==60.9.1 wheel==0.37.1 subunitreporter==22.2.0 From 58ccecff5414e7ceded6aaac3666e289aa54dd5b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 27 Apr 2023 11:17:19 -0400 Subject: [PATCH 22/38] Take a step towards unifying dependency pins used by tox env and Docker image building --- .circleci/populate-wheelhouse.sh | 21 +++-------------- setup.py | 39 +++++++++++++++++++++++++++++--- tox.ini | 19 +--------------- 3 files changed, 40 insertions(+), 39 deletions(-) diff --git a/.circleci/populate-wheelhouse.sh b/.circleci/populate-wheelhouse.sh index 374ca0adb..f103a6af8 100755 --- a/.circleci/populate-wheelhouse.sh +++ b/.circleci/populate-wheelhouse.sh @@ -3,18 +3,6 @@ # https://vaneyckt.io/posts/safer_bash_scripts_with_set_euxo_pipefail/ set -euxo pipefail -# Basic Python packages that you just need to have around to do anything, -# practically speaking. -BASIC_DEPS="pip wheel" - -# Python packages we need to support the test infrastructure. *Not* packages -# Tahoe-LAFS itself (implementation or test suite) need. -TEST_DEPS="tox~=3.0" - -# Python packages we need to generate test reports for CI infrastructure. -# *Not* packages Tahoe-LAFS itself (implement or test suite) need. -REPORTING_DEPS="python-subunit junitxml subunitreporter" - # The filesystem location of the wheelhouse which we'll populate with wheels # for all of our dependencies. WHEELHOUSE_PATH="$1" @@ -41,15 +29,12 @@ export PIP_FIND_LINKS="file://${WHEELHOUSE_PATH}" LANG="en_US.UTF-8" "${PIP}" \ wheel \ --wheel-dir "${WHEELHOUSE_PATH}" \ - "${PROJECT_ROOT}"[test] \ - ${BASIC_DEPS} \ - ${TEST_DEPS} \ - ${REPORTING_DEPS} + "${PROJECT_ROOT}"[testenv] \ + "${PROJECT_ROOT}"[test] # Not strictly wheelhouse population but ... Note we omit basic deps here. # They're in the wheelhouse if Tahoe-LAFS wants to drag them in but it will # have to ask. "${PIP}" \ install \ - ${TEST_DEPS} \ - ${REPORTING_DEPS} + "${PROJECT_ROOT}"[testenv] diff --git a/setup.py b/setup.py index 2418c6dbe..6e16381e6 100644 --- a/setup.py +++ b/setup.py @@ -398,10 +398,44 @@ setup(name="tahoe-lafs", # also set in __init__.py "dulwich", "gpg", ], + + # Here are the dependencies required to set up a reproducible test + # environment. This could be for CI or local development. These + # are *not* library dependencies of the test suite itself. They are + # the tools we use to run the test suite at all. + "testenv": [ + # Pin all of these versions for the same reason you ever want to + # pin anything: to prevent new releases with regressions from + # introducing spurious failures into CI runs for whatever + # development work is happening at the time. The versions + # selected here are just the current versions at the time. + # Bumping them to keep up with future releases is fine as long + # as those releases are known to actually work. + + # XXX For the moment, unpinned so we use whatever is in the + # image. The images vary in what versions they have. :/ + "pip", # ==22.0.3", + "wheel", # ==0.37.1" + "setuptools", # ==60.9.1", + "tox", # ~=3.0", + "subunitreporter", # ==22.2.0", + "python-subunit", # ==1.4.2", + "junitxml", # ==0.7", + "coverage", # ~= 5.0", + + # As an exception, we don't pin certifi because it contains CA + # certificates which necessarily change over time. Pinning this + # is guaranteed to cause things to break eventually as old + # certificates expire and as new ones are used in the wild that + # aren't present in whatever version we pin. Hopefully there + # won't be functionality regressions in new releases of this + # package that cause us the kind of suffering we're trying to + # avoid with the above pins. + "certifi", + ], + "test": [ - "coverage ~= 5.0", "mock", - "tox ~= 3.0", "pytest", "pytest-twisted", "hypothesis >= 3.6.1", @@ -410,7 +444,6 @@ setup(name="tahoe-lafs", # also set in __init__.py "fixtures", "beautifulsoup4", "html5lib", - "junitxml", # Pin old version until # https://github.com/paramiko/paramiko/issues/1961 is fixed. "paramiko < 2.9", diff --git a/tox.ini b/tox.ini index 6e56496d4..3b7a96503 100644 --- a/tox.ini +++ b/tox.ini @@ -30,24 +30,7 @@ passenv = TAHOE_LAFS_* PIP_* SUBUNITREPORTER_* USERPROFILE HOMEDRIVE HOMEPATH # available to those systems. Installing it ahead of time (with pip) avoids # this problem. deps = - # Pin all of these versions for the same reason you ever want to pin - # anything: to prevent new releases with regressions from introducing - # spurious failures into CI runs for whatever development work is - # happening at the time. The versions selected here are just the current - # versions at the time. Bumping them to keep up with future releases is - # fine as long as those releases are known to actually work. - pip==22.3.1 - setuptools==60.9.1 - wheel==0.37.1 - subunitreporter==22.2.0 - # As an exception, we don't pin certifi because it contains CA - # certificates which necessarily change over time. Pinning this is - # guaranteed to cause things to break eventually as old certificates - # expire and as new ones are used in the wild that aren't present in - # whatever version we pin. Hopefully there won't be functionality - # regressions in new releases of this package that cause us the kind of - # suffering we're trying to avoid with the above pins. - certifi + .[testenv] # We add usedevelop=False because testing against a true installation gives # more useful results. From 66d3de059432a3e1d12b14b50b66ac2ea263929d Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 27 Apr 2023 11:31:26 -0400 Subject: [PATCH 23/38] narrowly pin these dependencies This will break because these are not the versions on all Docker CI images but we need to pin them to rebuild those images with the correct versions. Rebuilding the images might break CI for all other branches. But! It's broken already, so it's not like it's any worse. --- setup.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/setup.py b/setup.py index 6e16381e6..127d17328 100644 --- a/setup.py +++ b/setup.py @@ -411,17 +411,14 @@ setup(name="tahoe-lafs", # also set in __init__.py # selected here are just the current versions at the time. # Bumping them to keep up with future releases is fine as long # as those releases are known to actually work. - - # XXX For the moment, unpinned so we use whatever is in the - # image. The images vary in what versions they have. :/ - "pip", # ==22.0.3", - "wheel", # ==0.37.1" - "setuptools", # ==60.9.1", - "tox", # ~=3.0", - "subunitreporter", # ==22.2.0", - "python-subunit", # ==1.4.2", - "junitxml", # ==0.7", - "coverage", # ~= 5.0", + "pip==22.0.3", + "wheel==0.37.1" + "setuptools==60.9.1", + "tox~=3.0", + "subunitreporter==22.2.0", + "python-subunit==1.4.2", + "junitxml==0.7", + "coverage ~= 5.0", # As an exception, we don't pin certifi because it contains CA # certificates which necessarily change over time. Pinning this From 29961a08b2c4097ee527043216a41b17a7da048d Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 27 Apr 2023 11:40:49 -0400 Subject: [PATCH 24/38] typo in the requirements list... --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 127d17328..b1d54b43c 100644 --- a/setup.py +++ b/setup.py @@ -412,7 +412,7 @@ setup(name="tahoe-lafs", # also set in __init__.py # Bumping them to keep up with future releases is fine as long # as those releases are known to actually work. "pip==22.0.3", - "wheel==0.37.1" + "wheel==0.37.1", "setuptools==60.9.1", "tox~=3.0", "subunitreporter==22.2.0", From f6e4e862a9d1bb8cc16d27b791e76aec09a298e6 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 28 Apr 2023 07:50:50 -0400 Subject: [PATCH 25/38] Require that the actual test run step do this part Keep this script to wheelhouse population. We might be giving up a tiny bit of performance here but let's make it work at all before we make it fast. --- .circleci/populate-wheelhouse.sh | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.circleci/populate-wheelhouse.sh b/.circleci/populate-wheelhouse.sh index f103a6af8..239c8367b 100755 --- a/.circleci/populate-wheelhouse.sh +++ b/.circleci/populate-wheelhouse.sh @@ -31,10 +31,3 @@ LANG="en_US.UTF-8" "${PIP}" \ --wheel-dir "${WHEELHOUSE_PATH}" \ "${PROJECT_ROOT}"[testenv] \ "${PROJECT_ROOT}"[test] - -# Not strictly wheelhouse population but ... Note we omit basic deps here. -# They're in the wheelhouse if Tahoe-LAFS wants to drag them in but it will -# have to ask. -"${PIP}" \ - install \ - "${PROJECT_ROOT}"[testenv] From 70caa22370b9646096e83cb18057287a3946698f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 28 Apr 2023 07:51:45 -0400 Subject: [PATCH 26/38] have to do certifi in tox.ini by the time setup.py is being processed it is too late for certifi to help --- setup.py | 10 ---------- tox.ini | 22 +++++++++++++++------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/setup.py b/setup.py index b1d54b43c..599eb5898 100644 --- a/setup.py +++ b/setup.py @@ -419,16 +419,6 @@ setup(name="tahoe-lafs", # also set in __init__.py "python-subunit==1.4.2", "junitxml==0.7", "coverage ~= 5.0", - - # As an exception, we don't pin certifi because it contains CA - # certificates which necessarily change over time. Pinning this - # is guaranteed to cause things to break eventually as old - # certificates expire and as new ones are used in the wild that - # aren't present in whatever version we pin. Hopefully there - # won't be functionality regressions in new releases of this - # package that cause us the kind of suffering we're trying to - # avoid with the above pins. - "certifi", ], "test": [ diff --git a/tox.ini b/tox.ini index 3b7a96503..609a78b13 100644 --- a/tox.ini +++ b/tox.ini @@ -23,14 +23,22 @@ minversion = 2.4 [testenv] passenv = TAHOE_LAFS_* PIP_* SUBUNITREPORTER_* USERPROFILE HOMEDRIVE HOMEPATH -# Get "certifi" to avoid bug #2913. Basically if a `setup_requires=...` causes -# a package to be installed (with setuptools) then it'll fail on certain -# platforms (travis's OX-X 10.12, Slackware 14.2) because PyPI's TLS -# requirements (TLS >= 1.2) are incompatible with the old TLS clients -# available to those systems. Installing it ahead of time (with pip) avoids -# this problem. deps = - .[testenv] + # We pull in certify *here* to avoid bug #2913. Basically if a + # `setup_requires=...` causes a package to be installed (with setuptools) + # then it'll fail on certain platforms (travis's OX-X 10.12, Slackware + # 14.2) because PyPI's TLS requirements (TLS >= 1.2) are incompatible with + # the old TLS clients available to those systems. Installing it ahead of + # time (with pip) avoids this problem. + # + # We don't pin an exact version of it because it contains CA certificates + # which necessarily change over time. Pinning this is guaranteed to cause + # things to break eventually as old certificates expire and as new ones + # are used in the wild that aren't present in whatever version we pin. + # Hopefully there won't be functionality regressions in new releases of + # this package that cause us the kind of suffering we're trying to avoid + # with the above pins. + certifi # We add usedevelop=False because testing against a true installation gives # more useful results. From 17706f582ee54532b7a117b3b97ffce3e0108b7a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 28 Apr 2023 07:52:05 -0400 Subject: [PATCH 27/38] use tox testenv `extras` to request testenv too --- tox.ini | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tox.ini b/tox.ini index 609a78b13..2edb15a0b 100644 --- a/tox.ini +++ b/tox.ini @@ -43,9 +43,14 @@ deps = # We add usedevelop=False because testing against a true installation gives # more useful results. usedevelop = False -# We use extras=test to get things like "mock" that are required for our unit -# tests. -extras = test + +extras = + # Get general testing environment dependencies so we can run the tests + # how we like. + testenv + + # And get all of the test suite's actual direct Python dependencies. + test setenv = # Define TEST_SUITE in the environment as an aid to constructing the From f48eb81d9d741857b6fe6cbabab0e04942238036 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 28 Apr 2023 07:57:51 -0400 Subject: [PATCH 28/38] restrict werkzeug more, at least for the moment --- setup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 599eb5898..d823029e2 100644 --- a/setup.py +++ b/setup.py @@ -141,8 +141,10 @@ install_requires = [ # HTTP server and client "klein", + # 2.2.0 has a bug: https://github.com/pallets/werkzeug/issues/2465 - "werkzeug != 2.2.0", + # 2.3.x has an incompatibility with Klein: https://github.com/twisted/klein/pull/575 + "werkzeug != 2.2.0, != 2.3.0, != 2.3.1", "treq", "cbor2", From f0b98aead562495f7f2019dd886b4e61c5fe68f9 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 28 Apr 2023 13:32:42 -0400 Subject: [PATCH 29/38] You don't need tox *inside* your test environment. You need tox to *manage* your test environment (this is the premise, at least). --- .circleci/setup-virtualenv.sh | 4 ++++ setup.py | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.circleci/setup-virtualenv.sh b/.circleci/setup-virtualenv.sh index 7087c5120..7fc6dc528 100755 --- a/.circleci/setup-virtualenv.sh +++ b/.circleci/setup-virtualenv.sh @@ -28,6 +28,10 @@ shift || : export PIP_FIND_LINKS="file://${WHEELHOUSE_PATH}" export PIP_NO_INDEX="1" +# Get tox inside the bootstrap virtualenv since we use tox to manage the rest +# of the environment. +"${BOOTSTRAP_VENV}"/bin/pip install "tox~=3.0" + # Get everything else installed in it, too. "${BOOTSTRAP_VENV}"/bin/tox \ -c "${PROJECT_ROOT}"/tox.ini \ diff --git a/setup.py b/setup.py index d823029e2..bac93a4bb 100644 --- a/setup.py +++ b/setup.py @@ -416,7 +416,6 @@ setup(name="tahoe-lafs", # also set in __init__.py "pip==22.0.3", "wheel==0.37.1", "setuptools==60.9.1", - "tox~=3.0", "subunitreporter==22.2.0", "python-subunit==1.4.2", "junitxml==0.7", From d67016d1b99ed1750c01af0caa4c137a768f31ce Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 28 Apr 2023 13:39:49 -0400 Subject: [PATCH 30/38] Get the right version of tox in the wheelhouse --- .circleci/populate-wheelhouse.sh | 3 ++- .circleci/setup-virtualenv.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.circleci/populate-wheelhouse.sh b/.circleci/populate-wheelhouse.sh index 239c8367b..f7ce361a8 100755 --- a/.circleci/populate-wheelhouse.sh +++ b/.circleci/populate-wheelhouse.sh @@ -30,4 +30,5 @@ LANG="en_US.UTF-8" "${PIP}" \ wheel \ --wheel-dir "${WHEELHOUSE_PATH}" \ "${PROJECT_ROOT}"[testenv] \ - "${PROJECT_ROOT}"[test] + "${PROJECT_ROOT}"[test] \ + "tox~=3.0" diff --git a/.circleci/setup-virtualenv.sh b/.circleci/setup-virtualenv.sh index 7fc6dc528..3f0074da3 100755 --- a/.circleci/setup-virtualenv.sh +++ b/.circleci/setup-virtualenv.sh @@ -30,7 +30,7 @@ export PIP_NO_INDEX="1" # Get tox inside the bootstrap virtualenv since we use tox to manage the rest # of the environment. -"${BOOTSTRAP_VENV}"/bin/pip install "tox~=3.0" +"${BOOTSTRAP_VENV}"/bin/pip install tox # Get everything else installed in it, too. "${BOOTSTRAP_VENV}"/bin/tox \ From a088b1d8125404b53f76cd7408d29df08cb9ab2a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 28 Apr 2023 13:49:14 -0400 Subject: [PATCH 31/38] don't bother to make a wheel of tox, just install it --- .circleci/populate-wheelhouse.sh | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.circleci/populate-wheelhouse.sh b/.circleci/populate-wheelhouse.sh index f7ce361a8..14d421652 100755 --- a/.circleci/populate-wheelhouse.sh +++ b/.circleci/populate-wheelhouse.sh @@ -30,5 +30,8 @@ LANG="en_US.UTF-8" "${PIP}" \ wheel \ --wheel-dir "${WHEELHOUSE_PATH}" \ "${PROJECT_ROOT}"[testenv] \ - "${PROJECT_ROOT}"[test] \ - "tox~=3.0" + "${PROJECT_ROOT}"[test] + +# Put tox right into the bootstrap environment because everyone is going to +# need to use it. +"${PIP}" install "tox~=3.0" From 29c0ca59748507f95035de4aff1faaa65995102b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 28 Apr 2023 13:51:22 -0400 Subject: [PATCH 32/38] put the tox installation near other software installation --- .circleci/create-virtualenv.sh | 4 ++++ .circleci/populate-wheelhouse.sh | 4 ---- .circleci/setup-virtualenv.sh | 4 ---- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/.circleci/create-virtualenv.sh b/.circleci/create-virtualenv.sh index 810ce5ae2..7327d0859 100755 --- a/.circleci/create-virtualenv.sh +++ b/.circleci/create-virtualenv.sh @@ -47,3 +47,7 @@ export PIP_FIND_LINKS="file://${WHEELHOUSE_PATH}" # above, it may still not be able to get us a compatible version unless we # explicitly ask for one. "${PIP}" install --upgrade setuptools==44.0.0 wheel + +# Just about every user of this image wants to use tox from the bootstrap +# virtualenv so go ahead and install it now. +"${PIP}" install "tox~=3.0" diff --git a/.circleci/populate-wheelhouse.sh b/.circleci/populate-wheelhouse.sh index 14d421652..239c8367b 100755 --- a/.circleci/populate-wheelhouse.sh +++ b/.circleci/populate-wheelhouse.sh @@ -31,7 +31,3 @@ LANG="en_US.UTF-8" "${PIP}" \ --wheel-dir "${WHEELHOUSE_PATH}" \ "${PROJECT_ROOT}"[testenv] \ "${PROJECT_ROOT}"[test] - -# Put tox right into the bootstrap environment because everyone is going to -# need to use it. -"${PIP}" install "tox~=3.0" diff --git a/.circleci/setup-virtualenv.sh b/.circleci/setup-virtualenv.sh index 3f0074da3..7087c5120 100755 --- a/.circleci/setup-virtualenv.sh +++ b/.circleci/setup-virtualenv.sh @@ -28,10 +28,6 @@ shift || : export PIP_FIND_LINKS="file://${WHEELHOUSE_PATH}" export PIP_NO_INDEX="1" -# Get tox inside the bootstrap virtualenv since we use tox to manage the rest -# of the environment. -"${BOOTSTRAP_VENV}"/bin/pip install tox - # Get everything else installed in it, too. "${BOOTSTRAP_VENV}"/bin/tox \ -c "${PROJECT_ROOT}"/tox.ini \ From 04ef5a02b2a27e7dc224b11f51615369d99f7e74 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 28 Apr 2023 14:12:01 -0400 Subject: [PATCH 33/38] eh ... these things moved into the tox-managed venv not intentional but not sure what a _good_ fix is, so try this. --- .circleci/run-tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/run-tests.sh b/.circleci/run-tests.sh index 6d7a881fe..b1e45af9b 100755 --- a/.circleci/run-tests.sh +++ b/.circleci/run-tests.sh @@ -93,5 +93,5 @@ if [ -n "${ARTIFACTS}" ]; then # Create a junitxml results area. mkdir -p "$(dirname "${JUNITXML}")" - "${BOOTSTRAP_VENV}"/bin/subunit2junitxml < "${SUBUNIT2}" > "${JUNITXML}" || "${alternative}" + "${BOOTSTRAP_VENV}"/.tox/"${TAHOE_LAFS_TOX_ENVIRONMENT}"/bin/subunit2junitxml < "${SUBUNIT2}" > "${JUNITXML}" || "${alternative}" fi From fa034781b46f2d3ae5351a9c57c7d55825fdebfe Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 28 Apr 2023 14:29:21 -0400 Subject: [PATCH 34/38] Perhaps this is the correct way to locate the tox-managed venv --- .circleci/run-tests.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.circleci/run-tests.sh b/.circleci/run-tests.sh index b1e45af9b..d897cc729 100755 --- a/.circleci/run-tests.sh +++ b/.circleci/run-tests.sh @@ -79,9 +79,10 @@ else alternative="false" fi +WORKDIR=/tmp/tahoe-lafs.tox ${TIMEOUT} ${BOOTSTRAP_VENV}/bin/tox \ -c ${PROJECT_ROOT}/tox.ini \ - --workdir /tmp/tahoe-lafs.tox \ + --workdir "${WORKDIR}" \ -e "${TAHOE_LAFS_TOX_ENVIRONMENT}" \ ${TAHOE_LAFS_TOX_ARGS} || "${alternative}" @@ -93,5 +94,6 @@ if [ -n "${ARTIFACTS}" ]; then # Create a junitxml results area. mkdir -p "$(dirname "${JUNITXML}")" - "${BOOTSTRAP_VENV}"/.tox/"${TAHOE_LAFS_TOX_ENVIRONMENT}"/bin/subunit2junitxml < "${SUBUNIT2}" > "${JUNITXML}" || "${alternative}" + + "${WORKDIR}/${TAHOE_LAFS_TOX_ENVIRONMENT}/bin/subunit2junitxml" < "${SUBUNIT2}" > "${JUNITXML}" || "${alternative}" fi From 3c660aff5d23849cdc6cdd788a455a14f2bb7881 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 1 May 2023 09:19:01 -0400 Subject: [PATCH 35/38] a comment about the other test extra --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index bac93a4bb..49004b1ff 100644 --- a/setup.py +++ b/setup.py @@ -422,6 +422,7 @@ setup(name="tahoe-lafs", # also set in __init__.py "coverage ~= 5.0", ], + # Here are the library dependencies of the test suite. "test": [ "mock", "pytest", From 0af84c9ac1fdf07cb644d381b693dfcafcdf594e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 1 May 2023 09:28:46 -0400 Subject: [PATCH 36/38] news fragment --- newsfragments/4020.installation | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/4020.installation diff --git a/newsfragments/4020.installation b/newsfragments/4020.installation new file mode 100644 index 000000000..8badf4b3c --- /dev/null +++ b/newsfragments/4020.installation @@ -0,0 +1 @@ +werkzeug 2.3.0 and werkzeug 2.3.1 are now blacklisted by the package metadata due to incompatibilities with klein. From b21b15f3954ae328fbaafd7d53839b62c4329d4b Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 1 May 2023 11:56:59 -0400 Subject: [PATCH 37/38] Blocking newer werkzeug is a temporary measure. --- newsfragments/4020.installation | 1 - newsfragments/4020.minor | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 newsfragments/4020.installation create mode 100644 newsfragments/4020.minor diff --git a/newsfragments/4020.installation b/newsfragments/4020.installation deleted file mode 100644 index 8badf4b3c..000000000 --- a/newsfragments/4020.installation +++ /dev/null @@ -1 +0,0 @@ -werkzeug 2.3.0 and werkzeug 2.3.1 are now blacklisted by the package metadata due to incompatibilities with klein. diff --git a/newsfragments/4020.minor b/newsfragments/4020.minor new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/newsfragments/4020.minor @@ -0,0 +1 @@ + From 4ca056b51c3dcc9f13f424bf133bd9dc34de8d93 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 1 May 2023 11:57:35 -0400 Subject: [PATCH 38/38] Be more general, 2.3.2 just came out for example. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 49004b1ff..4f28b4438 100644 --- a/setup.py +++ b/setup.py @@ -144,7 +144,7 @@ install_requires = [ # 2.2.0 has a bug: https://github.com/pallets/werkzeug/issues/2465 # 2.3.x has an incompatibility with Klein: https://github.com/twisted/klein/pull/575 - "werkzeug != 2.2.0, != 2.3.0, != 2.3.1", + "werkzeug != 2.2.0, < 2.3", "treq", "cbor2",