From 113eeb0e5908887aac8b03bd59a23fdc6999a3c5 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 4 May 2022 10:21:55 -0400 Subject: [PATCH 01/13] News file. --- newsfragments/3891.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3891.minor diff --git a/newsfragments/3891.minor b/newsfragments/3891.minor new file mode 100644 index 000000000..e69de29bb From c1ce74f88d346d92299af11c11ab01789d368c4e Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 4 May 2022 11:03:14 -0400 Subject: [PATCH 02/13] Ability to list shares, enabling more of IStorageClient to run over HTTP. --- docs/proposed/http-storage-node-protocol.rst | 2 +- src/allmydata/storage/http_client.py | 20 ++++++++++++ src/allmydata/storage/http_server.py | 14 ++++++++ src/allmydata/storage/server.py | 34 +++++++++++++------- src/allmydata/storage_client.py | 5 +-- src/allmydata/test/test_istorageserver.py | 1 - 6 files changed, 60 insertions(+), 16 deletions(-) diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst index 3926d9f4a..693ce9290 100644 --- a/docs/proposed/http-storage-node-protocol.rst +++ b/docs/proposed/http-storage-node-protocol.rst @@ -738,7 +738,7 @@ Reading ``GET /v1/mutable/:storage_index/shares`` !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -Retrieve a list indicating all shares available for the indicated storage index. +Retrieve a set indicating all shares available for the indicated storage index. For example:: [1, 5] diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index da350e0c6..5920d5a5b 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -106,6 +106,11 @@ _SCHEMAS = { share_number = uint """ ), + "mutable_list_shares": Schema( + """ + response = #6.258([* uint]) + """ + ), } @@ -720,3 +725,18 @@ class StorageClientMutables: return read_share_chunk( self._client, "mutable", storage_index, share_number, offset, length ) + + @async_to_deferred + async def list_shares(self, storage_index: bytes) -> set[int]: + """ + List the share numbers for a given storage index. + """ + # TODO unit test all the things + url = self._client.relative_url( + "/v1/mutable/{}/shares".format(_encode_si(storage_index)) + ) + response = await self._client.request("GET", url) + if response.code == http.OK: + return await _decode_cbor(response, _SCHEMAS["mutable_list_shares"]) + else: + raise ClientException(response.code) diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py index 0169d1463..0b407a1c4 100644 --- a/src/allmydata/storage/http_server.py +++ b/src/allmydata/storage/http_server.py @@ -645,6 +645,20 @@ class HTTPServer(object): ) return data + @_authorized_route( + _app, + set(), + "/v1/mutable//shares", + methods=["GET"], + ) + def list_mutable_shares(self, request, authorization, storage_index): + """List mutable shares for a storage index.""" + try: + shares = self._storage_server.list_mutable_shares(storage_index) + except KeyError: + raise _HTTPError(http.NOT_FOUND) + return self._send_encoded(request, shares) + @implementer(IStreamServerEndpoint) @attr.s diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py index 9d1a3d6a4..1a0255601 100644 --- a/src/allmydata/storage/server.py +++ b/src/allmydata/storage/server.py @@ -1,18 +1,9 @@ """ Ported to Python 3. """ -from __future__ import division -from __future__ import absolute_import -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import bytes_to_native_str, PY2 -if PY2: - # Omit open() to get native behavior where open("w") always accepts native - # strings. Omit bytes so we don't leak future's custom bytes. - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, pow, round, super, dict, list, object, range, str, max, min # noqa: F401 -else: - from typing import Dict, Tuple +from __future__ import annotations +from future.utils import bytes_to_native_str +from typing import Dict, Tuple import os, re @@ -699,6 +690,25 @@ class StorageServer(service.MultiService): self) return share + def list_mutable_shares(self, storage_index) -> set[int]: + """List all share numbers for the given mutable. + + Raises ``KeyError`` if the storage index is not known. + """ + # TODO unit test + si_dir = storage_index_to_dir(storage_index) + # shares exist if there is a file for them + bucketdir = os.path.join(self.sharedir, si_dir) + if not os.path.isdir(bucketdir): + raise KeyError("Not found") + result = set() + for sharenum_s in os.listdir(bucketdir): + try: + result.add(int(sharenum_s)) + except ValueError: + continue + return result + def slot_readv(self, storage_index, shares, readv): start = self._clock.seconds() self.count("readv") diff --git a/src/allmydata/storage_client.py b/src/allmydata/storage_client.py index 68164e697..8b2f68a9e 100644 --- a/src/allmydata/storage_client.py +++ b/src/allmydata/storage_client.py @@ -1196,9 +1196,10 @@ class _HTTPStorageServer(object): mutable_client = StorageClientMutables(self._http_client) pending_reads = {} reads = {} - # TODO if shares list is empty, that means list all shares, so we need + # If shares list is empty, that means list all shares, so we need # to do a query to get that. - assert shares # TODO replace with call to list shares if and only if it's empty + if not shares: + shares = yield mutable_client.list_shares(storage_index) # Start all the queries in parallel: for share_number in shares: diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py index e7b869713..d9fd13acb 100644 --- a/src/allmydata/test/test_istorageserver.py +++ b/src/allmydata/test/test_istorageserver.py @@ -1154,5 +1154,4 @@ class HTTPMutableAPIsTests( "test_add_lease_renewal", "test_add_new_lease", "test_advise_corrupt_share", - "test_slot_readv_no_shares", } From 852162ba0694f0405666d432d39b464f646eeca0 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 4 May 2022 11:03:35 -0400 Subject: [PATCH 03/13] More accurate docs. --- src/allmydata/storage/http_client.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index 5920d5a5b..2db28dc72 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -380,16 +380,14 @@ def read_share_chunk( """ Download a chunk of data from a share. - TODO https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3857 Failed - downloads should be transparently retried and redownloaded by the - implementation a few times so that if a failure percolates up, the - caller can assume the failure isn't a short-term blip. + TODO https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3857 Failed downloads + should be transparently retried and redownloaded by the implementation a + few times so that if a failure percolates up, the caller can assume the + failure isn't a short-term blip. - NOTE: the underlying HTTP protocol is much more flexible than this API, - so a future refactor may expand this in order to simplify the calling - code and perhaps download data more efficiently. But then again maybe - the HTTP protocol will be simplified, see - https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3777 + NOTE: the underlying HTTP protocol is somewhat more flexible than this API, + insofar as it doesn't always require a range. In practice a range is + always provided by the current callers. """ url = client.relative_url( "/v1/{}/{}/{}".format(share_type, _encode_si(storage_index), share_number) @@ -717,7 +715,7 @@ class StorageClientMutables: share_number: int, offset: int, length: int, - ) -> bytes: + ) -> Deferred[bytes]: """ Download a chunk of data from a share. """ From 06029d2878b6ad6edc67ae79dc1f59124c6934f0 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 4 May 2022 11:25:13 -0400 Subject: [PATCH 04/13] Another end-to-end test passing (albeit with ugly implementation). --- src/allmydata/storage/http_client.py | 6 +++++ src/allmydata/storage/http_server.py | 33 ++++++++++++++--------- src/allmydata/test/test_istorageserver.py | 1 - 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index 2db28dc72..0229bef03 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -706,6 +706,12 @@ class StorageClientMutables: if response.code == http.OK: result = await _decode_cbor(response, _SCHEMAS["mutable_read_test_write"]) return ReadTestWriteResult(success=result["success"], reads=result["data"]) + elif response.code == http.UNAUTHORIZED: + # TODO mabye we can fix this to be nicer at some point? Custom + # exception? + from foolscap.api import RemoteException + + raise RemoteException("Authorization failed") else: raise ClientException(response.code, (await response.content())) diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py index 0b407a1c4..748790a72 100644 --- a/src/allmydata/storage/http_server.py +++ b/src/allmydata/storage/http_server.py @@ -46,6 +46,7 @@ from .common import si_a2b from .immutable import BucketWriter, ConflictingWriteError from ..util.hashutil import timing_safe_compare from ..util.base32 import rfc3548_alphabet +from allmydata.interfaces import BadWriteEnablerError class ClientSecretsException(Exception): @@ -587,19 +588,25 @@ class HTTPServer(object): authorization[Secrets.LEASE_RENEW], authorization[Secrets.LEASE_CANCEL], ) - success, read_data = self._storage_server.slot_testv_and_readv_and_writev( - storage_index, - secrets, - { - k: ( - [(d["offset"], d["size"], b"eq", d["specimen"]) for d in v["test"]], - [(d["offset"], d["data"]) for d in v["write"]], - v["new-length"], - ) - for (k, v) in rtw_request["test-write-vectors"].items() - }, - [(d["offset"], d["size"]) for d in rtw_request["read-vector"]], - ) + try: + success, read_data = self._storage_server.slot_testv_and_readv_and_writev( + storage_index, + secrets, + { + k: ( + [ + (d["offset"], d["size"], b"eq", d["specimen"]) + for d in v["test"] + ], + [(d["offset"], d["data"]) for d in v["write"]], + v["new-length"], + ) + for (k, v) in rtw_request["test-write-vectors"].items() + }, + [(d["offset"], d["size"]) for d in rtw_request["read-vector"]], + ) + except BadWriteEnablerError: + raise _HTTPError(http.UNAUTHORIZED) return self._send_encoded(request, {"success": success, "data": read_data}) @_authorized_route( diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py index d9fd13acb..a3e75bbac 100644 --- a/src/allmydata/test/test_istorageserver.py +++ b/src/allmydata/test/test_istorageserver.py @@ -1150,7 +1150,6 @@ class HTTPMutableAPIsTests( # TODO will be implemented in later tickets SKIP_TESTS = { - "test_STARAW_write_enabler_must_match", "test_add_lease_renewal", "test_add_new_lease", "test_advise_corrupt_share", From 2833bec80e7e6ff069b4b6eee890f99942c3dfc4 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 5 May 2022 12:04:45 -0400 Subject: [PATCH 05/13] Unit test the new storage server backend API. --- src/allmydata/storage/server.py | 1 - src/allmydata/test/test_storage.py | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py index 1a0255601..b46303cd8 100644 --- a/src/allmydata/storage/server.py +++ b/src/allmydata/storage/server.py @@ -695,7 +695,6 @@ class StorageServer(service.MultiService): Raises ``KeyError`` if the storage index is not known. """ - # TODO unit test si_dir = storage_index_to_dir(storage_index) # shares exist if there is a file for them bucketdir = os.path.join(self.sharedir, si_dir) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index b37f74c24..8f1ece401 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -1315,6 +1315,31 @@ class MutableServer(unittest.TestCase): self.failUnless(isinstance(readv_data, dict)) self.failUnlessEqual(len(readv_data), 0) + def test_list_mutable_shares(self): + """ + ``StorageServer.list_mutable_shares()`` returns a set of share numbers + for the given storage index, or raises ``KeyError`` if it does not exist at all. + """ + ss = self.create("test_list_mutable_shares") + + # Initially, nothing exists: + with self.assertRaises(KeyError): + ss.list_mutable_shares(b"si1") + + self.allocate(ss, b"si1", b"we1", b"le1", [0, 1, 4, 2], 12) + shares0_1_2_4 = ss.list_mutable_shares(b"si1") + + # Remove share 2, by setting size to 0: + secrets = (self.write_enabler(b"we1"), + self.renew_secret(b"le1"), + self.cancel_secret(b"le1")) + ss.slot_testv_and_readv_and_writev(b"si1", secrets, {2: ([], [], 0)}, []) + shares0_1_4 = ss.list_mutable_shares(b"si1") + self.assertEqual( + (shares0_1_2_4, shares0_1_4), + ({0, 1, 2, 4}, {0, 1, 4}) + ) + def test_bad_magic(self): ss = self.create("test_bad_magic") self.allocate(ss, b"si1", b"we1", next(self._lease_secret), set([0]), 10) From b3fed56c00d03599b4a8479e5de0a36c696c7a97 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 5 May 2022 12:11:09 -0400 Subject: [PATCH 06/13] Move Foolscap compatibility to a better place. --- src/allmydata/storage/http_client.py | 6 ------ src/allmydata/storage_client.py | 16 +++++++++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index 0229bef03..2db28dc72 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -706,12 +706,6 @@ class StorageClientMutables: if response.code == http.OK: result = await _decode_cbor(response, _SCHEMAS["mutable_read_test_write"]) return ReadTestWriteResult(success=result["success"], reads=result["data"]) - elif response.code == http.UNAUTHORIZED: - # TODO mabye we can fix this to be nicer at some point? Custom - # exception? - from foolscap.api import RemoteException - - raise RemoteException("Authorization failed") else: raise ClientException(response.code, (await response.content())) diff --git a/src/allmydata/storage_client.py b/src/allmydata/storage_client.py index 8b2f68a9e..cd489a307 100644 --- a/src/allmydata/storage_client.py +++ b/src/allmydata/storage_client.py @@ -50,6 +50,7 @@ from zope.interface import ( Interface, implementer, ) +from twisted.web import http from twisted.internet import defer from twisted.application import service from twisted.plugin import ( @@ -78,7 +79,7 @@ from allmydata.util.dictutil import BytesKeyDict, UnicodeKeyDict from allmydata.storage.http_client import ( StorageClient, StorageClientImmutables, StorageClientGeneral, ClientException as HTTPClientException, StorageClientMutables, - ReadVector, TestWriteVectors, WriteVector, TestVector + ReadVector, TestWriteVectors, WriteVector, TestVector, ClientException ) @@ -1247,8 +1248,13 @@ class _HTTPStorageServer(object): ReadVector(offset=offset, size=size) for (offset, size) in r_vector ] - client_result = yield mutable_client.read_test_write_chunks( - storage_index, we_secret, lr_secret, lc_secret, client_tw_vectors, - client_read_vectors, - ) + try: + client_result = yield mutable_client.read_test_write_chunks( + storage_index, we_secret, lr_secret, lc_secret, client_tw_vectors, + client_read_vectors, + ) + except ClientException as e: + if e.code == http.UNAUTHORIZED: + raise RemoteException("Unauthorized write, possibly you passed the wrong write enabler?") + raise return (client_result.success, client_result.reads) From 5b0762d3a3a8fa1eb98aa6cd3b5b4d14e53047a3 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 10 May 2022 13:59:58 -0400 Subject: [PATCH 07/13] Workaround for autobahn issues. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index c84d0ecde..2b4fd6988 100644 --- a/setup.py +++ b/setup.py @@ -114,7 +114,7 @@ install_requires = [ "attrs >= 18.2.0", # WebSocket library for twisted and asyncio - "autobahn >= 19.5.2", + "autobahn < 22.4.1", # remove this when https://github.com/crossbario/autobahn-python/issues/1566 is fixed # Support for Python 3 transition "future >= 0.18.2", From 4afe3eb224d6f26ac768aaccbca68c69035d57d4 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 11 May 2022 10:58:13 -0400 Subject: [PATCH 08/13] Clarify sets vs lists some more. --- docs/proposed/http-storage-node-protocol.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst index 693ce9290..7e0b4a542 100644 --- a/docs/proposed/http-storage-node-protocol.rst +++ b/docs/proposed/http-storage-node-protocol.rst @@ -350,8 +350,10 @@ Because of the simple types used throughout and the equivalence described in `RFC 7049`_ these examples should be representative regardless of which of these two encodings is chosen. +The one exception is sets. For CBOR messages, any sequence that is semantically a set (i.e. no repeated values allowed, order doesn't matter, and elements are hashable in Python) should be sent as a set. Tag 6.258 is used to indicate sets in CBOR; see `the CBOR registry `_ for more details. +Sets will be represented as JSON lists in examples because JSON doesn't support sets. HTTP Design ~~~~~~~~~~~ @@ -739,7 +741,7 @@ Reading !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Retrieve a set indicating all shares available for the indicated storage index. -For example:: +For example (this is shown as list, since it will be list for JSON, but will be set for CBOR):: [1, 5] From 07e16b80b5df22a5620d390cb34032a55e62e795 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 11 May 2022 11:00:05 -0400 Subject: [PATCH 09/13] Better name. --- src/allmydata/storage/http_server.py | 4 ++-- src/allmydata/storage/server.py | 2 +- src/allmydata/test/test_storage.py | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py index 748790a72..96e906e43 100644 --- a/src/allmydata/storage/http_server.py +++ b/src/allmydata/storage/http_server.py @@ -658,10 +658,10 @@ class HTTPServer(object): "/v1/mutable//shares", methods=["GET"], ) - def list_mutable_shares(self, request, authorization, storage_index): + def enumerate_mutable_shares(self, request, authorization, storage_index): """List mutable shares for a storage index.""" try: - shares = self._storage_server.list_mutable_shares(storage_index) + shares = self._storage_server.enumerate_mutable_shares(storage_index) except KeyError: raise _HTTPError(http.NOT_FOUND) return self._send_encoded(request, shares) diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py index b46303cd8..ab7947bf9 100644 --- a/src/allmydata/storage/server.py +++ b/src/allmydata/storage/server.py @@ -690,7 +690,7 @@ class StorageServer(service.MultiService): self) return share - def list_mutable_shares(self, storage_index) -> set[int]: + def enumerate_mutable_shares(self, storage_index) -> set[int]: """List all share numbers for the given mutable. Raises ``KeyError`` if the storage index is not known. diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 8f1ece401..9bc218fa6 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -1315,26 +1315,26 @@ class MutableServer(unittest.TestCase): self.failUnless(isinstance(readv_data, dict)) self.failUnlessEqual(len(readv_data), 0) - def test_list_mutable_shares(self): + def test_enumerate_mutable_shares(self): """ - ``StorageServer.list_mutable_shares()`` returns a set of share numbers + ``StorageServer.enumerate_mutable_shares()`` returns a set of share numbers for the given storage index, or raises ``KeyError`` if it does not exist at all. """ - ss = self.create("test_list_mutable_shares") + ss = self.create("test_enumerate_mutable_shares") # Initially, nothing exists: with self.assertRaises(KeyError): - ss.list_mutable_shares(b"si1") + ss.enumerate_mutable_shares(b"si1") self.allocate(ss, b"si1", b"we1", b"le1", [0, 1, 4, 2], 12) - shares0_1_2_4 = ss.list_mutable_shares(b"si1") + shares0_1_2_4 = ss.enumerate_mutable_shares(b"si1") # Remove share 2, by setting size to 0: secrets = (self.write_enabler(b"we1"), self.renew_secret(b"le1"), self.cancel_secret(b"le1")) ss.slot_testv_and_readv_and_writev(b"si1", secrets, {2: ([], [], 0)}, []) - shares0_1_4 = ss.list_mutable_shares(b"si1") + shares0_1_4 = ss.enumerate_mutable_shares(b"si1") self.assertEqual( (shares0_1_2_4, shares0_1_4), ({0, 1, 2, 4}, {0, 1, 4}) From 6d412a017c34fc6f6528c89b1443d9bdf7caee64 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 11 May 2022 11:00:46 -0400 Subject: [PATCH 10/13] Type annotation. --- src/allmydata/storage/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py index ab7947bf9..f1b835780 100644 --- a/src/allmydata/storage/server.py +++ b/src/allmydata/storage/server.py @@ -690,7 +690,7 @@ class StorageServer(service.MultiService): self) return share - def enumerate_mutable_shares(self, storage_index) -> set[int]: + def enumerate_mutable_shares(self, storage_index: bytes) -> set[int]: """List all share numbers for the given mutable. Raises ``KeyError`` if the storage index is not known. From 4b62ec082bed7ecc33612741dfbffc16868ca3ff Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 11 May 2022 11:11:24 -0400 Subject: [PATCH 11/13] Match Foolscap behavior for slot_readv of unknown storage index. --- src/allmydata/storage_client.py | 8 +++++++- src/allmydata/test/test_istorageserver.py | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/allmydata/storage_client.py b/src/allmydata/storage_client.py index cd489a307..83a1233f5 100644 --- a/src/allmydata/storage_client.py +++ b/src/allmydata/storage_client.py @@ -1200,7 +1200,13 @@ class _HTTPStorageServer(object): # If shares list is empty, that means list all shares, so we need # to do a query to get that. if not shares: - shares = yield mutable_client.list_shares(storage_index) + try: + shares = yield mutable_client.list_shares(storage_index) + except ClientException as e: + if e.code == http.NOT_FOUND: + shares = set() + else: + raise # Start all the queries in parallel: for share_number in shares: diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py index a3e75bbac..66535ddda 100644 --- a/src/allmydata/test/test_istorageserver.py +++ b/src/allmydata/test/test_istorageserver.py @@ -854,6 +854,22 @@ class IStorageServerMutableAPIsTestsMixin(object): {0: [b"abcdefg"], 1: [b"0123456"], 2: [b"9876543"]}, ) + @inlineCallbacks + def test_slot_readv_unknown_storage_index(self): + """ + With unknown storage index, ``IStorageServer.slot_readv()`` TODO. + """ + storage_index = new_storage_index() + reads = yield self.storage_client.slot_readv( + storage_index, + shares=[], + readv=[(0, 7)], + ) + self.assertEqual( + reads, + {}, + ) + @inlineCallbacks def create_slot(self): """Create a slot with sharenum 0.""" From 457db8f992c62e8f5c5bc4cdcb6e99f097062f08 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Wed, 11 May 2022 11:17:57 -0400 Subject: [PATCH 12/13] Get rid of the "no such storage index" edge case, since it's not really necessary. --- src/allmydata/storage/http_server.py | 5 +---- src/allmydata/storage/server.py | 7 ++----- src/allmydata/storage_client.py | 8 +------- src/allmydata/test/test_storage.py | 12 ++++++------ 4 files changed, 10 insertions(+), 22 deletions(-) diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py index 96e906e43..a1641f563 100644 --- a/src/allmydata/storage/http_server.py +++ b/src/allmydata/storage/http_server.py @@ -660,10 +660,7 @@ class HTTPServer(object): ) def enumerate_mutable_shares(self, request, authorization, storage_index): """List mutable shares for a storage index.""" - try: - shares = self._storage_server.enumerate_mutable_shares(storage_index) - except KeyError: - raise _HTTPError(http.NOT_FOUND) + shares = self._storage_server.enumerate_mutable_shares(storage_index) return self._send_encoded(request, shares) diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py index f1b835780..bcf44dc30 100644 --- a/src/allmydata/storage/server.py +++ b/src/allmydata/storage/server.py @@ -691,15 +691,12 @@ class StorageServer(service.MultiService): return share def enumerate_mutable_shares(self, storage_index: bytes) -> set[int]: - """List all share numbers for the given mutable. - - Raises ``KeyError`` if the storage index is not known. - """ + """Return all share numbers for the given mutable.""" si_dir = storage_index_to_dir(storage_index) # shares exist if there is a file for them bucketdir = os.path.join(self.sharedir, si_dir) if not os.path.isdir(bucketdir): - raise KeyError("Not found") + return set() result = set() for sharenum_s in os.listdir(bucketdir): try: diff --git a/src/allmydata/storage_client.py b/src/allmydata/storage_client.py index 83a1233f5..cd489a307 100644 --- a/src/allmydata/storage_client.py +++ b/src/allmydata/storage_client.py @@ -1200,13 +1200,7 @@ class _HTTPStorageServer(object): # If shares list is empty, that means list all shares, so we need # to do a query to get that. if not shares: - try: - shares = yield mutable_client.list_shares(storage_index) - except ClientException as e: - if e.code == http.NOT_FOUND: - shares = set() - else: - raise + shares = yield mutable_client.list_shares(storage_index) # Start all the queries in parallel: for share_number in shares: diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 9bc218fa6..65d09de25 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -1317,14 +1317,14 @@ class MutableServer(unittest.TestCase): def test_enumerate_mutable_shares(self): """ - ``StorageServer.enumerate_mutable_shares()`` returns a set of share numbers - for the given storage index, or raises ``KeyError`` if it does not exist at all. + ``StorageServer.enumerate_mutable_shares()`` returns a set of share + numbers for the given storage index, or an empty set if it does not + exist at all. """ ss = self.create("test_enumerate_mutable_shares") # Initially, nothing exists: - with self.assertRaises(KeyError): - ss.enumerate_mutable_shares(b"si1") + empty = ss.enumerate_mutable_shares(b"si1") self.allocate(ss, b"si1", b"we1", b"le1", [0, 1, 4, 2], 12) shares0_1_2_4 = ss.enumerate_mutable_shares(b"si1") @@ -1336,8 +1336,8 @@ class MutableServer(unittest.TestCase): ss.slot_testv_and_readv_and_writev(b"si1", secrets, {2: ([], [], 0)}, []) shares0_1_4 = ss.enumerate_mutable_shares(b"si1") self.assertEqual( - (shares0_1_2_4, shares0_1_4), - ({0, 1, 2, 4}, {0, 1, 4}) + (empty, shares0_1_2_4, shares0_1_4), + (set(), {0, 1, 2, 4}, {0, 1, 4}) ) def test_bad_magic(self): From da4deab167187ec4baf02f84da8e8ae7e03a6a8a Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 16 May 2022 11:19:46 -0400 Subject: [PATCH 13/13] Note version with fix. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 2b4fd6988..d07031cd9 100644 --- a/setup.py +++ b/setup.py @@ -114,7 +114,7 @@ install_requires = [ "attrs >= 18.2.0", # WebSocket library for twisted and asyncio - "autobahn < 22.4.1", # remove this when https://github.com/crossbario/autobahn-python/issues/1566 is fixed + "autobahn < 22.4.1", # remove this when 22.4.3 is released # Support for Python 3 transition "future >= 0.18.2",