From 5d9a1a5ab5fe09774dfb3dc4e3bc06d511952ca4 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 25 Sep 2020 11:46:31 -0400 Subject: [PATCH 1/7] Some progress towards passing tests. --- src/allmydata/test/test_helper.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/allmydata/test/test_helper.py b/src/allmydata/test/test_helper.py index 3774704c6..0b198bb35 100644 --- a/src/allmydata/test/test_helper.py +++ b/src/allmydata/test/test_helper.py @@ -1,3 +1,5 @@ +from future.builtins import str + import os from twisted.internet import defer from twisted.trial import unittest @@ -18,7 +20,7 @@ from .common import ( MiB = 1024*1024 -DATA = "I need help\n" * 1000 +DATA = b"I need help\n" * 1000 class CHKUploadHelper_fake(offloaded.CHKUploadHelper): def start_encrypted(self, eu): @@ -33,8 +35,8 @@ class CHKUploadHelper_fake(offloaded.CHKUploadHelper): "segment_size": segsize, "size": size, } - ueb_hash = "fake" - v = uri.CHKFileVerifierURI(self._storage_index, "x"*32, + ueb_hash = b"fake" + v = uri.CHKFileVerifierURI(self._storage_index, b"x"*32, needed_shares, total_shares, size) _UR = upload.UploadResults ur = _UR(file_size=size, @@ -56,7 +58,7 @@ class CHKUploadHelper_fake(offloaded.CHKUploadHelper): class Helper_fake_upload(offloaded.Helper): def _make_chk_upload_helper(self, storage_index, lp): - si_s = si_b2a(storage_index) + si_s = str(si_b2a(storage_index), "utf-8") incoming_file = os.path.join(self._chk_incoming, si_s) encoding_file = os.path.join(self._chk_encoding, si_s) uh = CHKUploadHelper_fake(storage_index, self, @@ -69,7 +71,7 @@ class Helper_fake_upload(offloaded.Helper): class Helper_already_uploaded(Helper_fake_upload): def _check_chk(self, storage_index, lp): res = upload.HelperUploadResults() - res.uri_extension_hash = hashutil.uri_extension_hash("") + res.uri_extension_hash = hashutil.uri_extension_hash(b"") # we're pretending that the file they're trying to upload was already # present in the grid. We return some information about the file, so @@ -127,7 +129,7 @@ class AssistedUpload(unittest.TestCase): lambda h: self.tub, EMPTY_CLIENT_CONFIG, ) - self.s.secret_holder = client.SecretHolder("lease secret", "converge") + self.s.secret_holder = client.SecretHolder(b"lease secret", b"converge") self.s.startService() t.setServiceParent(self.s) @@ -162,11 +164,11 @@ class AssistedUpload(unittest.TestCase): def _ready(res): assert u._helper - return upload_data(u, DATA, convergence="some convergence string") + return upload_data(u, DATA, convergence=b"some convergence string") d.addCallback(_ready) def _uploaded(results): the_uri = results.get_uri() - assert "CHK" in the_uri + assert b"CHK" in the_uri d.addCallback(_uploaded) def _check_empty(res): @@ -195,11 +197,11 @@ class AssistedUpload(unittest.TestCase): # this must be a multiple of 'required_shares'==k segsize = mathutil.next_multiple(segsize, k) - key = hashutil.convergence_hash(k, n, segsize, DATA, "test convergence string") + key = hashutil.convergence_hash(k, n, segsize, DATA, b"test convergence string") assert len(key) == 16 encryptor = aes.create_encryptor(key) SI = hashutil.storage_index_hash(key) - SI_s = si_b2a(SI) + SI_s = str(si_b2a(SI), "utf-8") encfile = os.path.join(self.basedir, "CHK_encoding", SI_s) f = open(encfile, "wb") f.write(aes.encrypt_data(encryptor, DATA)) @@ -212,7 +214,7 @@ class AssistedUpload(unittest.TestCase): def _ready(res): assert u._helper - return upload_data(u, DATA, convergence="test convergence string") + return upload_data(u, DATA, convergence=b"test convergence string") d.addCallback(_ready) def _uploaded(results): the_uri = results.get_uri() @@ -239,11 +241,11 @@ class AssistedUpload(unittest.TestCase): def _ready(res): assert u._helper - return upload_data(u, DATA, convergence="some convergence string") + return upload_data(u, DATA, convergence=b"some convergence string") d.addCallback(_ready) def _uploaded(results): the_uri = results.get_uri() - assert "CHK" in the_uri + assert b"CHK" in the_uri d.addCallback(_uploaded) def _check_empty(res): From d8c9affccbf993f867b4f8a9d17a5f3adac64822 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 25 Sep 2020 13:25:04 -0400 Subject: [PATCH 2/7] Bunch more places that need to be bytes for protocol backwards compatibility. --- src/allmydata/interfaces.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py index 43565dc4f..c7cd2370d 100644 --- a/src/allmydata/interfaces.py +++ b/src/allmydata/interfaces.py @@ -2836,17 +2836,17 @@ class RIControlClient(RemoteInterface): # debug stuff - def upload_random_data_from_file(size=int, convergence=str): + def upload_random_data_from_file(size=int, convergence=bytes): return str - def download_to_tempfile_and_delete(uri=str): + def download_to_tempfile_and_delete(uri=bytes): return None def get_memory_usage(): """Return a dict describes the amount of memory currently in use. The keys are 'VmPeak', 'VmSize', and 'VmData'. The values are integers, measuring memory consupmtion in bytes.""" - return DictOf(str, int) + return DictOf(bytes, int) def speed_test(count=int, size=int, mutable=Any()): """Write 'count' tempfiles to disk, all of the given size. Measure @@ -2871,7 +2871,7 @@ class RIControlClient(RemoteInterface): return DictOf(str, float) -UploadResults = Any() #DictOf(str, str) +UploadResults = Any() #DictOf(bytes, bytes) class RIEncryptedUploadable(RemoteInterface): @@ -2884,7 +2884,7 @@ class RIEncryptedUploadable(RemoteInterface): return (int, int, int, long) def read_encrypted(offset=Offset, length=ReadSize): - return ListOf(str) + return ListOf(bytes) def close(): return None @@ -2897,7 +2897,7 @@ class RICHKUploadHelper(RemoteInterface): """ Return a dictionary of version information. """ - return DictOf(str, Any()) + return DictOf(bytes, Any()) def upload(reader=RIEncryptedUploadable): return UploadResults @@ -2910,7 +2910,7 @@ class RIHelper(RemoteInterface): """ Return a dictionary of version information. """ - return DictOf(str, Any()) + return DictOf(bytes, Any()) def upload_chk(si=StorageIndex): """See if a file with a given storage index needs uploading. The @@ -2944,7 +2944,7 @@ class RIStatsProvider(RemoteInterface): stats are instantaneous measures (potentially time averaged internally) """ - return DictOf(str, DictOf(str, ChoiceOf(float, int, long, None))) + return DictOf(bytes, DictOf(bytes, ChoiceOf(float, int, long, None))) class RIStatsGatherer(RemoteInterface): @@ -2953,7 +2953,7 @@ class RIStatsGatherer(RemoteInterface): Provides a monitoring service for centralised collection of stats """ - def provide(provider=RIStatsProvider, nickname=str): + def provide(provider=RIStatsProvider, nickname=bytes): """ @param provider: a stats collector instance that should be polled periodically by the gatherer to collect stats. @@ -2965,7 +2965,7 @@ class RIStatsGatherer(RemoteInterface): class IStatsProducer(Interface): def get_stats(): """ - returns a dictionary, with str keys representing the names of stats + returns a dictionary, with bytes keys representing the names of stats to be monitored, and numeric values. """ From 565c48045e1dd0b685bdd352f584569fc7360ce4 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 25 Sep 2020 13:28:59 -0400 Subject: [PATCH 3/7] Closer to passing tests. --- src/allmydata/immutable/offloaded.py | 6 +++--- src/allmydata/immutable/upload.py | 6 +++--- src/allmydata/test/test_helper.py | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/allmydata/immutable/offloaded.py b/src/allmydata/immutable/offloaded.py index e04e94e8f..fb8c706a3 100644 --- a/src/allmydata/immutable/offloaded.py +++ b/src/allmydata/immutable/offloaded.py @@ -203,7 +203,7 @@ class CHKUploadHelper(Referenceable, upload.CHKUploader): def _finished(self, ur): assert interfaces.IUploadResults.providedBy(ur), ur vcapstr = ur.get_verifycapstr() - precondition(isinstance(vcapstr, str), vcapstr) + precondition(isinstance(vcapstr, bytes), vcapstr) v = uri.from_string(vcapstr) f_times = self._fetcher.get_times() @@ -492,9 +492,9 @@ class Helper(Referenceable): # helper at random. name = "helper" - VERSION = { "http://allmydata.org/tahoe/protocols/helper/v1" : + VERSION = { b"http://allmydata.org/tahoe/protocols/helper/v1" : { }, - "application-version": str(allmydata.__full_version__), + b"application-version": allmydata.__full_version__.encode("utf-8"), } MAX_UPLOAD_STATUSES = 10 diff --git a/src/allmydata/immutable/upload.py b/src/allmydata/immutable/upload.py index 1ab312ab6..5e38ba31a 100644 --- a/src/allmydata/immutable/upload.py +++ b/src/allmydata/immutable/upload.py @@ -1816,15 +1816,15 @@ class Uploader(service.MultiService, log.PrefixingLogMixin): def _got_helper(self, helper): self.log("got helper connection, getting versions") - default = { "http://allmydata.org/tahoe/protocols/helper/v1" : + default = { b"http://allmydata.org/tahoe/protocols/helper/v1" : { }, - "application-version": b"unknown: no get_version()", + b"application-version": b"unknown: no get_version()", } d = add_version_to_remote_reference(helper, default) d.addCallback(self._got_versioned_helper) def _got_versioned_helper(self, helper): - needed = "http://allmydata.org/tahoe/protocols/helper/v1" + needed = b"http://allmydata.org/tahoe/protocols/helper/v1" if needed not in helper.version: raise InsufficientVersionError(needed, helper.version) self._helper = helper diff --git a/src/allmydata/test/test_helper.py b/src/allmydata/test/test_helper.py index 0b198bb35..0c98e4af8 100644 --- a/src/allmydata/test/test_helper.py +++ b/src/allmydata/test/test_helper.py @@ -218,7 +218,7 @@ class AssistedUpload(unittest.TestCase): d.addCallback(_ready) def _uploaded(results): the_uri = results.get_uri() - assert "CHK" in the_uri + assert b"CHK" in the_uri d.addCallback(_uploaded) def _check_empty(res): From d19ae1e511b961805dfcce5352d6c68ec3bfe9ab Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 25 Sep 2020 14:00:18 -0400 Subject: [PATCH 4/7] Apparently __remote_name__ needs to be a native string. --- src/allmydata/interfaces.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py index c7cd2370d..7adc016c6 100644 --- a/src/allmydata/interfaces.py +++ b/src/allmydata/interfaces.py @@ -8,7 +8,7 @@ from __future__ import division from __future__ import print_function from __future__ import unicode_literals -from future.utils import PY2 +from future.utils import PY2, native_str if PY2: # Don't import object/str/dict/etc. types, so we don't break any # interfaces. Not importing open() because it triggers bogus flake8 error. @@ -105,7 +105,7 @@ ReadData = ListOf(ShareData) class RIStorageServer(RemoteInterface): - __remote_name__ = b"RIStorageServer.tahoe.allmydata.com" + __remote_name__ = native_str("RIStorageServer.tahoe.allmydata.com") def get_version(): """ @@ -1231,7 +1231,7 @@ class IMutableFileNode(IFileNode): the old contents), a 'first_time' boolean, and a servermap. As with download_best_version(), the old contents will be from the best recoverable version, but the modifier can use the servermap to make - other decisions (such as refusing to apply the delta if there are +1 other decisions (such as refusing to apply the delta if there are multiple parallel versions, or if there is evidence of a newer unrecoverable version). 'first_time' will be True the first time the modifier is called, and False on any subsequent calls. @@ -2875,7 +2875,7 @@ UploadResults = Any() #DictOf(bytes, bytes) class RIEncryptedUploadable(RemoteInterface): - __remote_name__ = b"RIEncryptedUploadable.tahoe.allmydata.com" + __remote_name__ = native_str("RIEncryptedUploadable.tahoe.allmydata.com") def get_size(): return Offset @@ -2891,7 +2891,7 @@ class RIEncryptedUploadable(RemoteInterface): class RICHKUploadHelper(RemoteInterface): - __remote_name__ = b"RIUploadHelper.tahoe.allmydata.com" + __remote_name__ = native_str("RIUploadHelper.tahoe.allmydata.com") def get_version(): """ @@ -2904,7 +2904,7 @@ class RICHKUploadHelper(RemoteInterface): class RIHelper(RemoteInterface): - __remote_name__ = b"RIHelper.tahoe.allmydata.com" + __remote_name__ = native_str("RIHelper.tahoe.allmydata.com") def get_version(): """ @@ -2931,7 +2931,7 @@ class RIHelper(RemoteInterface): class RIStatsProvider(RemoteInterface): - __remote_name__ = b"RIStatsProvider.tahoe.allmydata.com" + __remote_name__ = native_str("RIStatsProvider.tahoe.allmydata.com") """ Provides access to statistics and monitoring information. """ @@ -2948,7 +2948,7 @@ class RIStatsProvider(RemoteInterface): class RIStatsGatherer(RemoteInterface): - __remote_name__ = b"RIStatsGatherer.tahoe.allmydata.com" + __remote_name__ = native_str("RIStatsGatherer.tahoe.allmydata.com") """ Provides a monitoring service for centralised collection of stats """ From 21e3b355ecdd31828295110615aa4b61eb8a07ef Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 25 Sep 2020 14:03:25 -0400 Subject: [PATCH 5/7] Finish porting to Python 3. --- newsfragments/3446.minor | 0 src/allmydata/test/test_helper.py | 11 +++++++++-- src/allmydata/util/_python3.py | 1 + 3 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 newsfragments/3446.minor diff --git a/newsfragments/3446.minor b/newsfragments/3446.minor new file mode 100644 index 000000000..e69de29bb diff --git a/src/allmydata/test/test_helper.py b/src/allmydata/test/test_helper.py index 0c98e4af8..c47da3277 100644 --- a/src/allmydata/test/test_helper.py +++ b/src/allmydata/test/test_helper.py @@ -1,4 +1,11 @@ -from future.builtins import str +from __future__ import absolute_import +from __future__ import division +from __future__ import print_function +from __future__ import unicode_literals + +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 twisted.internet import defer @@ -136,7 +143,7 @@ class AssistedUpload(unittest.TestCase): self.s.tub = t # we never actually use this for network traffic, so it can use a # bogus host/port - t.setLocation("bogus:1234") + t.setLocation(b"bogus:1234") def setUpHelper(self, basedir, helper_class=Helper_fake_upload): fileutil.make_dirs(basedir) diff --git a/src/allmydata/util/_python3.py b/src/allmydata/util/_python3.py index fc2530f15..6347b5fc2 100644 --- a/src/allmydata/util/_python3.py +++ b/src/allmydata/util/_python3.py @@ -90,6 +90,7 @@ PORTED_TEST_MODULES = [ "allmydata.test.test_happiness", "allmydata.test.test_hashtree", "allmydata.test.test_hashutil", + "allmydata.test.test_helper", "allmydata.test.test_humanreadable", "allmydata.test.test_iputil", "allmydata.test.test_log", From f796b8a7da14dade8d04cd228d8ddd740bcece17 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 28 Sep 2020 10:02:43 -0400 Subject: [PATCH 6/7] Fix typo. --- src/allmydata/interfaces.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py index 7adc016c6..5c47bf663 100644 --- a/src/allmydata/interfaces.py +++ b/src/allmydata/interfaces.py @@ -1231,7 +1231,7 @@ class IMutableFileNode(IFileNode): the old contents), a 'first_time' boolean, and a servermap. As with download_best_version(), the old contents will be from the best recoverable version, but the modifier can use the servermap to make -1 other decisions (such as refusing to apply the delta if there are + other decisions (such as refusing to apply the delta if there are multiple parallel versions, or if there is evidence of a newer unrecoverable version). 'first_time' will be True the first time the modifier is called, and False on any subsequent calls. From d7b5230f0ebf600ccf9e039b3254bc038def800f Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Mon, 28 Sep 2020 10:06:06 -0400 Subject: [PATCH 7/7] Explain why __remote_name__ is a native string. --- src/allmydata/interfaces.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py index 5c47bf663..49dcf7646 100644 --- a/src/allmydata/interfaces.py +++ b/src/allmydata/interfaces.py @@ -2,6 +2,8 @@ Interfaces for Tahoe-LAFS. Ported to Python 3. + +Note that for RemoteInterfaces, the __remote_name__ needs to be a native string because of https://github.com/warner/foolscap/blob/43f4485a42c9c28e2c79d655b3a9e24d4e6360ca/src/foolscap/remoteinterface.py#L67 """ from __future__ import absolute_import from __future__ import division