From d142ccb159c70da4b1c1e4159f955f06b5deb7c2 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 4 Feb 2020 06:46:08 -0500 Subject: [PATCH 01/41] Use twisted.web.template in web/storage.py Related to ticket:3247. Nevow usage has been removed, and generated page looks the same as its former self, but tests are failing because test_storage.py assumes that we're using nevow. --- src/allmydata/web/storage.py | 258 ++++++++++++++----------- src/allmydata/web/storage_status.xhtml | 60 +++--- 2 files changed, 182 insertions(+), 136 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 79c0a8c38..3000fb70d 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -1,10 +1,10 @@ import time, json -from nevow import rend, tags as T +from twisted.python.filepath import FilePath +from twisted.web.template import tags as T, renderer, Element, renderElement, XMLFile from allmydata.web.common import ( - getxmlfile, abbreviate_time, - MultiFormatPage, + MultiFormatResource ) from allmydata.util.abbreviate import abbreviate_space from allmydata.util import time_format, idlib @@ -16,91 +16,100 @@ def remove_prefix(s, prefix): return s[len(prefix):] -class StorageStatus(MultiFormatPage): - docFactory = getxmlfile("storage_status.xhtml") - # the default 'data' argument is the StorageServer instance +class StorageStatusElement(Element): + loader = XMLFile(FilePath(__file__).sibling("storage_status.xhtml")) - def __init__(self, storage, nickname=""): - rend.Page.__init__(self, storage) + def __init__(self, storage, nickname): + super(StorageStatusElement, self).__init__() self.storage = storage - self.nickname = nickname + self.nick = nickname - def render_JSON(self, req): - req.setHeader("content-type", "text/plain") - d = {"stats": self.storage.get_stats(), - "bucket-counter": self.storage.bucket_counter.get_state(), - "lease-checker": self.storage.lease_checker.get_state(), - "lease-checker-progress": self.storage.lease_checker.get_progress(), - } - return json.dumps(d, indent=1) + "\n" + @renderer + def nickname(self, req, tag): + return self.nick - def data_nickname(self, ctx, storage): - return self.nickname - def data_nodeid(self, ctx, storage): + @renderer + def nodeid(self, req, tag): return idlib.nodeid_b2a(self.storage.my_nodeid) - def render_storage_running(self, ctx, storage): - if storage: - return ctx.tag - else: - return T.h1["No Storage Server Running"] + def get_stat(self, key): + return self.storage.get_stats().get(key) - def render_bool(self, ctx, data): - return {True: "Yes", False: "No"}[bool(data)] + def str(self, tag, val): + if val is None: + return tag("?") + return tag(str(val)) - def render_abbrev_space(self, ctx, size): - if size is None: - return "?" - return abbreviate_space(size) + def abbr(self, tag, val): + if val is None: + return tag("?") + return tag(abbreviate_space(val)) - def render_space(self, ctx, size): - if size is None: - return "?" - return "%d" % size + @renderer + def disk_total(self, req, tag): + return self.str(tag, self.get_stat("storage_server.disk_total")) - def data_stats(self, ctx, data): - # FYI: 'data' appears to be self, rather than the StorageServer - # object in self.original that gets passed to render_* methods. I - # still don't understand Nevow. + @renderer + def disk_total_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.disk_total")) - # Nevow has nevow.accessors.DictionaryContainer: Any data= directive - # that appears in a context in which the current data is a dictionary - # will be looked up as keys in that dictionary. So if data_stats() - # returns a dictionary, then we can use something like this: - # - # + @renderer + def disk_used(self, req, tag): + return self.str(tag, self.get_stat("storage_server.disk_used")) - # to use get_stats()["storage_server.disk_total"] . However, - # DictionaryContainer does a raw d[] instead of d.get(), so any - # missing keys will cause an error, even if the renderer can tolerate - # None values. To overcome this, we either need a dict-like object - # that always returns None for unknown keys, or we must pre-populate - # our dict with those missing keys, or we should get rid of data_ - # methods that return dicts (or find some way to override Nevow's - # handling of dictionaries). + @renderer + def disk_used_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.disk_used")) - d = dict([ (remove_prefix(k, "storage_server."), v) - for k,v in self.storage.get_stats().items() ]) - d.setdefault("disk_total", None) - d.setdefault("disk_used", None) - d.setdefault("disk_free_for_root", None) - d.setdefault("disk_free_for_nonroot", None) - d.setdefault("reserved_space", None) - d.setdefault("disk_avail", None) - return d + @renderer + def disk_free_for_root(self, req, tag): + return self.str(tag, self.get_stat("storage_server.disk_free_for_root")) - def data_last_complete_bucket_count(self, ctx, data): + @renderer + def disk_free_for_root_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.disk_free_for_root")) + + @renderer + def disk_free_for_nonroot(self, req, tag): + return self.str(tag, self.get_stat("storage_server.disk_free_for_nonroot")) + + @renderer + def disk_free_for_nonroot_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.disk_free_for_nonroot")) + + @renderer + def reserved_space(self, req, tag): + return self.str(tag, self.get_stat("storage_server.reserved_space")) + + @renderer + def reserved_space_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.reserved_space")) + + @renderer + def disk_avail(self, req, tag): + return self.str(tag, self.get_stat("storage_server.disk_avail")) + + @renderer + def disk_avail_abbrev(self, req, tag): + return self.abbr(tag, self.get_stat("storage_server.disk_avail")) + + @renderer + def accepting_immutable_shares(self, req, tag): + accepting = self.get_stat("storage_server.accepting_immutable_shares") + return {True: "Yes", False: "No"}[bool(accepting)] + + @renderer + def last_complete_bucket_count(self, req, tag): s = self.storage.bucket_counter.get_state() count = s.get("last-complete-bucket-count") if count is None: return "Not computed yet" - return count + return str(count) - def render_count_crawler_status(self, ctx, storage): + @renderer + def count_crawler_status(self, req, tag): p = self.storage.bucket_counter.get_progress() - return ctx.tag[self.format_crawler_progress(p)] + return self.format_crawler_progress(p) def format_crawler_progress(self, p): cycletime = p["estimated-time-per-cycle"] @@ -127,55 +136,51 @@ class StorageStatus(MultiFormatPage): return ["Next crawl in %s" % abbreviate_time(soon), cycletime_s] - def render_lease_expiration_enabled(self, ctx, data): + @renderer + def storage_running(self, req, tag): + if self.storage: + return tag + return tag("No Storage Server Running") + + @renderer + def lease_expiration_enabled(self, req, tag): lc = self.storage.lease_checker if lc.expiration_enabled: - return ctx.tag["Enabled: expired leases will be removed"] + return tag("Enabled: expired leases will be removed") else: - return ctx.tag["Disabled: scan-only mode, no leases will be removed"] + return tag("Disabled: scan-only mode, no leases will be removed") - def render_lease_expiration_mode(self, ctx, data): + @renderer + def lease_expiration_mode(self, req, tag): lc = self.storage.lease_checker if lc.mode == "age": if lc.override_lease_duration is None: - ctx.tag["Leases will expire naturally, probably 31 days after " - "creation or renewal."] + tag("Leases will expire naturally, probably 31 days after " + "creation or renewal.") else: - ctx.tag["Leases created or last renewed more than %s ago " - "will be considered expired." - % abbreviate_time(lc.override_lease_duration)] + tag("Leases created or last renewed more than %s ago " + "will be considered expired." + % abbreviate_time(lc.override_lease_duration)) else: assert lc.mode == "cutoff-date" localizedutcdate = time.strftime("%d-%b-%Y", time.gmtime(lc.cutoff_date)) isoutcdate = time_format.iso_utc_date(lc.cutoff_date) - ctx.tag["Leases created or last renewed before %s (%s) UTC " - "will be considered expired." % (isoutcdate, localizedutcdate, )] + tag("Leases created or last renewed before %s (%s) UTC " + "will be considered expired." + % (isoutcdate, localizedutcdate, )) if len(lc.mode) > 2: - ctx.tag[" The following sharetypes will be expired: ", - " ".join(sorted(lc.sharetypes_to_expire)), "."] - return ctx.tag + tag(" The following sharetypes will be expired: ", + " ".join(sorted(lc.sharetypes_to_expire)), ".") + return tag - def format_recovered(self, sr, a): - def maybe(d): - if d is None: - return "?" - return "%d" % d - return "%s shares, %s buckets (%s mutable / %s immutable), %s (%s / %s)" % \ - (maybe(sr["%s-shares" % a]), - maybe(sr["%s-buckets" % a]), - maybe(sr["%s-buckets-mutable" % a]), - maybe(sr["%s-buckets-immutable" % a]), - abbreviate_space(sr["%s-diskbytes" % a]), - abbreviate_space(sr["%s-diskbytes-mutable" % a]), - abbreviate_space(sr["%s-diskbytes-immutable" % a]), - ) - - def render_lease_current_cycle_progress(self, ctx, data): + @renderer + def lease_current_cycle_progress(self, req, tag): lc = self.storage.lease_checker p = lc.get_progress() - return ctx.tag[self.format_crawler_progress(p)] + return tag(self.format_crawler_progress(p)) - def render_lease_current_cycle_results(self, ctx, data): + @renderer + def lease_current_cycle_results(self, req, tag): lc = self.storage.lease_checker p = lc.get_progress() if not p["cycle-in-progress"]: @@ -229,10 +234,10 @@ class StorageStatus(MultiFormatPage): T.ul[ [T.li[ ["SI %s shnum %d" % corrupt_share for corrupt_share in so_far["corrupt-shares"] ] ]]]) + return tag("Current cycle:", p) - return ctx.tag["Current cycle:", p] - - def render_lease_last_cycle_results(self, ctx, data): + @renderer + def lease_last_cycle_results(self, req, tag): lc = self.storage.lease_checker h = lc.get_state()["history"] if not h: @@ -240,15 +245,15 @@ class StorageStatus(MultiFormatPage): last = h[max(h.keys())] start, end = last["cycle-start-finish-times"] - ctx.tag["Last complete cycle (which took %s and finished %s ago)" - " recovered: " % (abbreviate_time(end-start), - abbreviate_time(time.time() - end)), - self.format_recovered(last["space-recovered"], "actual") - ] + tag("Last complete cycle (which took %s and finished %s ago)" + " recovered: " % (abbreviate_time(end-start), + abbreviate_time(time.time() - end)), + self.format_recovered(last["space-recovered"], "actual")) p = T.ul() + def add(*pieces): - p[T.li[pieces]] + p(T.li(pieces)) saw = self.format_recovered(last["space-recovered"], "examined") add("and saw a total of ", saw) @@ -264,4 +269,37 @@ class StorageStatus(MultiFormatPage): for corrupt_share in last["corrupt-shares"] ] ]]]) - return ctx.tag[p] + return tag(p) + + def format_recovered(self, sr, a): + def maybe(d): + if d is None: + return "?" + return "%d" % d + return "%s shares, %s buckets (%s mutable / %s immutable), %s (%s / %s)" % \ + (maybe(sr["%s-shares" % a]), + maybe(sr["%s-buckets" % a]), + maybe(sr["%s-buckets-mutable" % a]), + maybe(sr["%s-buckets-immutable" % a]), + abbreviate_space(sr["%s-diskbytes" % a]), + abbreviate_space(sr["%s-diskbytes-mutable" % a]), + abbreviate_space(sr["%s-diskbytes-immutable" % a]), + ) + +class StorageStatus(MultiFormatResource): + def __init__(self, storage, nickname=""): + super(StorageStatus, self).__init__() + self.storage = storage + self.nickname = nickname + + def render_HTML(self, req): + return renderElement(req, StorageStatusElement(self.storage, self.nickname)) + + def render_JSON(self, req): + req.setHeader("content-type", "text/plain") + d = {"stats": self.storage.get_stats(), + "bucket-counter": self.storage.bucket_counter.get_state(), + "lease-checker": self.storage.lease_checker.get_state(), + "lease-checker-progress": self.storage.lease_checker.get_progress(), + } + return json.dumps(d, indent=1) + "\n" diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index d97daf9af..cfd7a860c 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -1,4 +1,4 @@ - + Tahoe-LAFS - Storage Server Status @@ -7,19 +7,19 @@ -
+

Storage Server Status

- +
- - + + - - + + - - + + - - + + - - + + - - + +
Total disk space:()()
Disk space used:- ()- ()
@@ -28,18 +28,18 @@
Disk space free (root):()() [see 1]
Disk space free (non-root):()() [see 2]
Reserved space:- ()- ()
@@ -48,23 +48,31 @@
Space Available to Tahoe:()()
    -
  • Server Nickname:
    • -
  • Server Nodeid:
    • -
  • Accepting new shares: -
    • +
  • Server Nickname: + + + +
    • +
  • Server Nodeid: + + + +
    • +
  • Accepting new shares: +
  • Total buckets: - + (the number of files and directories for which this server is holding a share)
      -
    • +
@@ -72,11 +80,11 @@

Lease Expiration Crawler

    -
  • Expiration
    • -
  • -
  • -
  • -
  • +
  • Expiration
    • +
  • +
  • +
  • +
From b29652e0f0d27aafd52771b3f7b57f511fcec076 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 4 Feb 2020 22:59:45 -0500 Subject: [PATCH 02/41] Add `StorageStatus::renderSynchronously` Related to ticket:3247 test_storage.py wants a `StorageStatus::renderSynchronously()` method and a `StorageStatus::renderHTTP()` method. Let us begin with the goofy first-cut. Both these methods are not only wrong, but they will also not please the test suite. However error messages produced in CI can be shared, and that way I can hopefully get unstuck. --- src/allmydata/web/storage.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 3000fb70d..8f36cf44d 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -1,7 +1,8 @@ import time, json from twisted.python.filepath import FilePath -from twisted.web.template import tags as T, renderer, Element, renderElement, XMLFile +from twisted.web.template import tags as T, \ + renderer, Element, renderElement, XMLFile from allmydata.web.common import ( abbreviate_time, MultiFormatResource @@ -303,3 +304,14 @@ class StorageStatus(MultiFormatResource): "lease-checker-progress": self.storage.lease_checker.get_progress(), } return json.dumps(d, indent=1) + "\n" + + def renderSynchronously(self): + # to appease the test suite. + elem = StorageStatusElement(self.storage, self.nickname) + result = [] + flattenString(None, elem).addCallback(result.append) + return result + + def renderHTTP(self, ctx=None): + # to appease the test suite. + self.renderSynchronously() From d3790a4d4227f31f06a35042448a1a119fa4658e Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 4 Feb 2020 23:16:48 -0500 Subject: [PATCH 03/41] Add missing `flattenString` import --- src/allmydata/web/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 8f36cf44d..a912d79f6 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -2,7 +2,7 @@ import time, json from twisted.python.filepath import FilePath from twisted.web.template import tags as T, \ - renderer, Element, renderElement, XMLFile + renderer, Element, renderElement, XMLFile, flattenString from allmydata.web.common import ( abbreviate_time, MultiFormatResource From c019c7e9556e31c599997ce6463fe679668c3a73 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Wed, 5 Feb 2020 22:05:33 -0500 Subject: [PATCH 04/41] Second version of renderSynchronously --- src/allmydata/web/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index a912d79f6..4ad5a2baf 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -310,7 +310,7 @@ class StorageStatus(MultiFormatResource): elem = StorageStatusElement(self.storage, self.nickname) result = [] flattenString(None, elem).addCallback(result.append) - return result + return result[0] def renderHTTP(self, ctx=None): # to appease the test suite. From 4e81a3a0a2906a2e2c3774a126afa750b7f9bf3c Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Wed, 5 Feb 2020 22:09:16 -0500 Subject: [PATCH 05/41] Check storage server status before using it --- src/allmydata/web/storage.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 4ad5a2baf..9a99286bf 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -31,9 +31,13 @@ class StorageStatusElement(Element): @renderer def nodeid(self, req, tag): + if not self.storage: + return tag("No storage server running.") return idlib.nodeid_b2a(self.storage.my_nodeid) def get_stat(self, key): + if not self.storage: + return None return self.storage.get_stats().get(key) def str(self, tag, val): @@ -101,6 +105,8 @@ class StorageStatusElement(Element): @renderer def last_complete_bucket_count(self, req, tag): + if not self.storage: + return tag("No storage server running.") s = self.storage.bucket_counter.get_state() count = s.get("last-complete-bucket-count") if count is None: @@ -109,6 +115,8 @@ class StorageStatusElement(Element): @renderer def count_crawler_status(self, req, tag): + if not self.storage: + return tag("No storage server running.") p = self.storage.bucket_counter.get_progress() return self.format_crawler_progress(p) @@ -145,6 +153,8 @@ class StorageStatusElement(Element): @renderer def lease_expiration_enabled(self, req, tag): + if not self.storage: + return tag("No storage server running.") lc = self.storage.lease_checker if lc.expiration_enabled: return tag("Enabled: expired leases will be removed") @@ -153,6 +163,8 @@ class StorageStatusElement(Element): @renderer def lease_expiration_mode(self, req, tag): + if not self.storage: + return tag("No storage server running.") lc = self.storage.lease_checker if lc.mode == "age": if lc.override_lease_duration is None: @@ -176,12 +188,16 @@ class StorageStatusElement(Element): @renderer def lease_current_cycle_progress(self, req, tag): + if not self.storage: + return tag("No storage server running.") lc = self.storage.lease_checker p = lc.get_progress() return tag(self.format_crawler_progress(p)) @renderer def lease_current_cycle_results(self, req, tag): + if not self.storage: + return tag("No storage server running.") lc = self.storage.lease_checker p = lc.get_progress() if not p["cycle-in-progress"]: @@ -239,6 +255,8 @@ class StorageStatusElement(Element): @renderer def lease_last_cycle_results(self, req, tag): + if not self.storage: + return tag("No storage server running.") lc = self.storage.lease_checker h = lc.get_state()["history"] if not h: From c88c97aad540b928129944bdcd349df6c7df72de Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Wed, 5 Feb 2020 23:09:39 -0500 Subject: [PATCH 06/41] Use right syntax for twisted.web.template tags --- src/allmydata/web/storage.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 9a99286bf..1e206d8cf 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -212,7 +212,7 @@ class StorageStatusElement(Element): p = T.ul() def add(*pieces): - p[T.li[pieces]] + p(T.li(pieces)) def maybe(d): if d is None: @@ -248,9 +248,9 @@ class StorageStatusElement(Element): if so_far["corrupt-shares"]: add("Corrupt shares:", - T.ul[ [T.li[ ["SI %s shnum %d" % corrupt_share + T.ul( (T.li( ["SI %s shnum %d" % corrupt_share for corrupt_share in so_far["corrupt-shares"] ] - ]]]) + )))) return tag("Current cycle:", p) @renderer @@ -284,9 +284,9 @@ class StorageStatusElement(Element): if last["corrupt-shares"]: add("Corrupt shares:", - T.ul[ [T.li[ ["SI %s shnum %d" % corrupt_share + T.ul( (T.li( ["SI %s shnum %d" % corrupt_share for corrupt_share in last["corrupt-shares"] ] - ]]]) + )))) return tag(p) From c7a63f957d4a476092f3bcff70e57dacc9d47c60 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 05:17:35 -0500 Subject: [PATCH 07/41] Refactor so that test_util pass --- src/allmydata/test/test_storage.py | 5 +++-- src/allmydata/web/storage.py | 10 ++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 9f3aee9b8..85f4dd2bd 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -30,7 +30,8 @@ from allmydata.interfaces import BadWriteEnablerError from allmydata.test.common import LoggingServiceParent, ShouldFailMixin from allmydata.test.common_web import WebRenderingMixin from allmydata.test.no_network import NoNetworkServer -from allmydata.web.storage import StorageStatus, remove_prefix +from allmydata.web.storage import StorageStatus, StorageStatusElement, \ + remove_prefix from allmydata.storage_client import ( _StorageServer, ) @@ -4208,7 +4209,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnlessIn("Reserved space: - 10.00 MB (10000000)", s) def test_util(self): - w = StorageStatus(None) + w = StorageStatusElement(None, None) self.failUnlessEqual(w.render_space(None, None), "?") self.failUnlessEqual(w.render_space(None, 10e6), "10000000") self.failUnlessEqual(w.render_abbrev_space(None, None), "?") diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 1e206d8cf..6b033bc5a 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -50,6 +50,16 @@ class StorageStatusElement(Element): return tag("?") return tag(abbreviate_space(val)) + def render_abbrev_space(self, ctx, size): + if size is None: + return "?" + return abbreviate_space(size) + + def render_space(self, ctx, size): + if size is None: + return "?" + return "%d" % size + @renderer def disk_total(self, req, tag): return self.str(tag, self.get_stat("storage_server.disk_total")) From 0993e610468afe97876c9e4af4367ac9a6a7bde4 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 05:20:45 -0500 Subject: [PATCH 08/41] Drop unused `ctx` argument from render_space methods --- src/allmydata/test/test_storage.py | 8 ++++---- src/allmydata/web/storage.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 85f4dd2bd..6f4c455f2 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -4210,9 +4210,9 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): def test_util(self): w = StorageStatusElement(None, None) - self.failUnlessEqual(w.render_space(None, None), "?") - self.failUnlessEqual(w.render_space(None, 10e6), "10000000") - self.failUnlessEqual(w.render_abbrev_space(None, None), "?") - self.failUnlessEqual(w.render_abbrev_space(None, 10e6), "10.00 MB") + self.failUnlessEqual(w.render_space(None), "?") + self.failUnlessEqual(w.render_space(10e6), "10000000") + self.failUnlessEqual(w.render_abbrev_space(None), "?") + self.failUnlessEqual(w.render_abbrev_space(10e6), "10.00 MB") self.failUnlessEqual(remove_prefix("foo.bar", "foo."), "bar") self.failUnlessEqual(remove_prefix("foo.bar", "baz."), None) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 6b033bc5a..88f099b24 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -50,12 +50,12 @@ class StorageStatusElement(Element): return tag("?") return tag(abbreviate_space(val)) - def render_abbrev_space(self, ctx, size): + def render_abbrev_space(self, size): if size is None: return "?" return abbreviate_space(size) - def render_space(self, ctx, size): + def render_space(self, size): if size is None: return "?" return "%d" % size From d46df30bd059633d68dee3eb5d42ba5e375d0d5c Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 05:49:11 -0500 Subject: [PATCH 09/41] Use render_space methods to render space --- src/allmydata/web/storage.py | 46 +++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 88f099b24..496766b92 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -40,16 +40,6 @@ class StorageStatusElement(Element): return None return self.storage.get_stats().get(key) - def str(self, tag, val): - if val is None: - return tag("?") - return tag(str(val)) - - def abbr(self, tag, val): - if val is None: - return tag("?") - return tag(abbreviate_space(val)) - def render_abbrev_space(self, size): if size is None: return "?" @@ -62,51 +52,63 @@ class StorageStatusElement(Element): @renderer def disk_total(self, req, tag): - return self.str(tag, self.get_stat("storage_server.disk_total")) + val = self.get_stat("storage_server.disk_total") + return tag(self.render_space(val)) @renderer def disk_total_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.disk_total")) + val = self.get_stat("storage_server.disk_total") + return tag(self.render_abbrev_space(val)) @renderer def disk_used(self, req, tag): - return self.str(tag, self.get_stat("storage_server.disk_used")) + val = self.get_stat("storage_server.disk_used") + return tag(self.render_space(val)) @renderer def disk_used_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.disk_used")) + val = self.get_stat("storage_server.disk_used") + return tag(self.render_abbrev_space(val)) @renderer def disk_free_for_root(self, req, tag): - return self.str(tag, self.get_stat("storage_server.disk_free_for_root")) + val = self.get_stat("storage_server.disk_free_for_root") + return tag(self.render_space(val)) @renderer def disk_free_for_root_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.disk_free_for_root")) + val = self.get_stat("storage_server.disk_free_for_root") + return tag(self.render_abbrev_space(val)) @renderer def disk_free_for_nonroot(self, req, tag): - return self.str(tag, self.get_stat("storage_server.disk_free_for_nonroot")) + val = self.get_stat("storage_server.disk_free_for_nonroot") + return tag(self.render_space(val)) @renderer def disk_free_for_nonroot_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.disk_free_for_nonroot")) + val = self.get_stat("storage_server.disk_free_for_nonroot") + return tag(self.render_abbrev_space(val)) @renderer def reserved_space(self, req, tag): - return self.str(tag, self.get_stat("storage_server.reserved_space")) + val = self.get_stat("storage_server.reserved_space") + return tag(self.render_space(val)) @renderer def reserved_space_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.reserved_space")) + val = self.get_stat("storage_server.reserved_space") + return tag(self.render_abbrev_space(val)) @renderer def disk_avail(self, req, tag): - return self.str(tag, self.get_stat("storage_server.disk_avail")) + val = self.get_stat("storage_server.disk_avail") + return tag(self.render_space(val)) @renderer def disk_avail_abbrev(self, req, tag): - return self.abbr(tag, self.get_stat("storage_server.disk_avail")) + val = self.get_stat("storage_server.disk_avail") + return tag(self.render_abbrev_space(val)) @renderer def accepting_immutable_shares(self, req, tag): From b14f36082c148119cc9814c56a02a4155447032a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 06:22:25 -0500 Subject: [PATCH 10/41] Use within table cells --- src/allmydata/web/storage_status.xhtml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index cfd7a860c..6e0e26008 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -13,13 +13,13 @@ - - + + - - + + - - + + - - + + - - + + - - + +
Total disk space:()()
Disk space used:- ()- ()
@@ -28,18 +28,18 @@
Disk space free (root):()() [see 1]
Disk space free (non-root):()() [see 2]
Reserved space:- ()- ()
@@ -48,8 +48,8 @@
Space Available to Tahoe:()()
From 2df2ae92d56722ee9d810a678d9b74fe197cb1ab Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 07:34:55 -0500 Subject: [PATCH 11/41] Fix test_storage.WebStatus.test_no_server failure --- src/allmydata/web/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 496766b92..c2e517dce 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -161,7 +161,7 @@ class StorageStatusElement(Element): def storage_running(self, req, tag): if self.storage: return tag - return tag("No Storage Server Running") + return T.h1("No Storage Server Running") @renderer def lease_expiration_enabled(self, req, tag): From b1c78244abeab83e0061e7586b26d4b44911a85d Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 08:19:29 -0500 Subject: [PATCH 12/41] Use parentheses in import statement --- src/allmydata/web/storage.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index c2e517dce..d45541457 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -1,8 +1,14 @@ import time, json from twisted.python.filepath import FilePath -from twisted.web.template import tags as T, \ - renderer, Element, renderElement, XMLFile, flattenString +from twisted.web.template import ( + Element, + XMLFile, + tags as T, + renderer, + renderElement, + flattenString +) from allmydata.web.common import ( abbreviate_time, MultiFormatResource From d3ff578640f85181fffd787b5209e7847033d8e9 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 09:55:33 -0500 Subject: [PATCH 13/41] Use parentheses in test suite's import statement --- src/allmydata/test/test_storage.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 6f4c455f2..6956fc757 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -30,8 +30,11 @@ from allmydata.interfaces import BadWriteEnablerError from allmydata.test.common import LoggingServiceParent, ShouldFailMixin from allmydata.test.common_web import WebRenderingMixin from allmydata.test.no_network import NoNetworkServer -from allmydata.web.storage import StorageStatus, StorageStatusElement, \ +from allmydata.web.storage import ( + StorageStatus, + StorageStatusElement, remove_prefix +) from allmydata.storage_client import ( _StorageServer, ) From 227d06fe64533ba45b1319b7f498e0df3492f670 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 11 Feb 2020 17:02:14 -0500 Subject: [PATCH 14/41] Add docstrings to StorageStatusElement --- src/allmydata/web/storage.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index d45541457..df8354a38 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -24,9 +24,15 @@ def remove_prefix(s, prefix): class StorageStatusElement(Element): + """Class to render a storage status page.""" + loader = XMLFile(FilePath(__file__).sibling("storage_status.xhtml")) - def __init__(self, storage, nickname): + def __init__(self, storage, nickname=""): + """ + :param _StorageServer storage: data about storage. + :param string nickname: friendly name for storage. + """ super(StorageStatusElement, self).__init__() self.storage = storage self.nick = nickname From f22417e51b4fadbba09cf2ad882605c5aecaef20 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Wed, 12 Feb 2020 12:14:24 -0500 Subject: [PATCH 15/41] Rename function for clarity --- src/allmydata/web/storage.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index df8354a38..a5d65fa85 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -47,7 +47,7 @@ class StorageStatusElement(Element): return tag("No storage server running.") return idlib.nodeid_b2a(self.storage.my_nodeid) - def get_stat(self, key): + def _get_storage_stat(self, key): if not self.storage: return None return self.storage.get_stats().get(key) @@ -64,67 +64,67 @@ class StorageStatusElement(Element): @renderer def disk_total(self, req, tag): - val = self.get_stat("storage_server.disk_total") + val = self._get_storage_stat("storage_server.disk_total") return tag(self.render_space(val)) @renderer def disk_total_abbrev(self, req, tag): - val = self.get_stat("storage_server.disk_total") + val = self._get_storage_stat("storage_server.disk_total") return tag(self.render_abbrev_space(val)) @renderer def disk_used(self, req, tag): - val = self.get_stat("storage_server.disk_used") + val = self._get_storage_stat("storage_server.disk_used") return tag(self.render_space(val)) @renderer def disk_used_abbrev(self, req, tag): - val = self.get_stat("storage_server.disk_used") + val = self._get_storage_stat("storage_server.disk_used") return tag(self.render_abbrev_space(val)) @renderer def disk_free_for_root(self, req, tag): - val = self.get_stat("storage_server.disk_free_for_root") + val = self._get_storage_stat("storage_server.disk_free_for_root") return tag(self.render_space(val)) @renderer def disk_free_for_root_abbrev(self, req, tag): - val = self.get_stat("storage_server.disk_free_for_root") + val = self._get_storage_stat("storage_server.disk_free_for_root") return tag(self.render_abbrev_space(val)) @renderer def disk_free_for_nonroot(self, req, tag): - val = self.get_stat("storage_server.disk_free_for_nonroot") + val = self._get_storage_stat("storage_server.disk_free_for_nonroot") return tag(self.render_space(val)) @renderer def disk_free_for_nonroot_abbrev(self, req, tag): - val = self.get_stat("storage_server.disk_free_for_nonroot") + val = self._get_storage_stat("storage_server.disk_free_for_nonroot") return tag(self.render_abbrev_space(val)) @renderer def reserved_space(self, req, tag): - val = self.get_stat("storage_server.reserved_space") + val = self._get_storage_stat("storage_server.reserved_space") return tag(self.render_space(val)) @renderer def reserved_space_abbrev(self, req, tag): - val = self.get_stat("storage_server.reserved_space") + val = self._get_storage_stat("storage_server.reserved_space") return tag(self.render_abbrev_space(val)) @renderer def disk_avail(self, req, tag): - val = self.get_stat("storage_server.disk_avail") + val = self._get_storage_stat("storage_server.disk_avail") return tag(self.render_space(val)) @renderer def disk_avail_abbrev(self, req, tag): - val = self.get_stat("storage_server.disk_avail") + val = self._get_storage_stat("storage_server.disk_avail") return tag(self.render_abbrev_space(val)) @renderer def accepting_immutable_shares(self, req, tag): - accepting = self.get_stat("storage_server.accepting_immutable_shares") + accepting = self._get_storage_stat("storage_server.accepting_immutable_shares") return {True: "Yes", False: "No"}[bool(accepting)] @renderer From 6e9a4e30d733c2a08d801df03f1107d9177ec76e Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Wed, 12 Feb 2020 12:15:26 -0500 Subject: [PATCH 16/41] Add a docstring --- src/allmydata/web/storage.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index a5d65fa85..263fa9344 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -48,6 +48,30 @@ class StorageStatusElement(Element): return idlib.nodeid_b2a(self.storage.my_nodeid) def _get_storage_stat(self, key): + """Get storage server statistics. + + Storage Server keeps a dict that contains various usage and + latency statistics. The dict looks like this: + + { + 'storage_server.accepting_immutable_shares': 1, + 'storage_server.allocated': 0, + 'storage_server.disk_avail': 106539192320, + 'storage_server.disk_free_for_nonroot': 106539192320, + 'storage_server.disk_free_for_root': 154415284224, + 'storage_server.disk_total': 941088460800, + 'storage_server.disk_used': 786673176576, + 'storage_server.latencies.add-lease.01_0_percentile': None, + 'storage_server.latencies.add-lease.10_0_percentile': None, + ... + } + + ``StorageServer.get_stats()`` returns the above dict. Storage + status page uses a subset of the items in the dict, concerning + disk usage. + + :param str key: storage server statistic we want to know. + """ if not self.storage: return None return self.storage.get_stats().get(key) From 9b51bdf7faca3314e5dff2de22b8e035b9114fec Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 12:25:20 -0500 Subject: [PATCH 17/41] Return result from renderHTTP Bogus renderHTTP, but test failures are now down to three. We just need to handle requests for JSON now. --- src/allmydata/web/storage.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 263fa9344..1656df70b 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -378,6 +378,18 @@ class StorageStatus(MultiFormatResource): flattenString(None, elem).addCallback(result.append) return result[0] + # to appease the test suite def renderHTTP(self, ctx=None): - # to appease the test suite. - self.renderSynchronously() + """Send HTML or JSON formatted data, based on request. + + This function contains a bit of nevow-ism, but since this is + only called from the test suite, the nevow-ism should go away + as we update things. + + :param _nevow.context.WovenContext ctx: context is passed on + from the test suite. We get a request out of this + context, and use the request to render a result. + + """ + from nevow.inevow import IRequest + return self.render(IRequest(ctx)) From 7a053ddeff85d32376c12582ca35a825e708704a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 15:29:42 -0500 Subject: [PATCH 18/41] Use explicit `None`-check on self.storage --- src/allmydata/web/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 1656df70b..76a09fda3 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -43,7 +43,7 @@ class StorageStatusElement(Element): @renderer def nodeid(self, req, tag): - if not self.storage: + if self.storage is None: return tag("No storage server running.") return idlib.nodeid_b2a(self.storage.my_nodeid) From e2fc1fc07ea56cd6b4805864d1d275170e0c261e Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 16:49:15 -0500 Subject: [PATCH 19/41] Move `renderSynchronously` to test suite --- src/allmydata/test/test_storage.py | 36 +++++++++++++++++++----------- src/allmydata/web/storage.py | 10 +-------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 6956fc757..f37401cc2 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -4,6 +4,7 @@ from twisted.trial import unittest from twisted.internet import defer from twisted.application import service +from twisted.web.template import flattenString from foolscap.api import fireEventually import itertools from allmydata import interfaces @@ -2963,6 +2964,15 @@ def remove_tags(s): s = re.sub(r'\s+', ' ', s) return s +def renderSynchronously(ss): + """ + :param _StorageStatus ss: a StorageStatus instance. + """ + elem = StorageStatusElement(ss.storage, ss.nickname) + result = [] + flattenString(None, elem).addCallback(result.append) + return result[0] + class MyBucketCountingCrawler(BucketCountingCrawler): def finished_prefix(self, cycle, prefix): BucketCountingCrawler.finished_prefix(self, cycle, prefix) @@ -2999,7 +3009,7 @@ class BucketCounter(unittest.TestCase, pollmixin.PollMixin): w = StorageStatus(ss) # this sample is before the crawler has started doing anything - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Accepting new shares: Yes", s) @@ -3022,7 +3032,7 @@ class BucketCounter(unittest.TestCase, pollmixin.PollMixin): self.failUnlessEqual(state["last-complete-prefix"], ss.bucket_counter.prefixes[0]) ss.bucket_counter.cpu_slice = 100.0 # finish as fast as possible - html = w.renderSynchronously() + html = renderSynchronously(w) s = remove_tags(html) self.failUnlessIn(" Current crawl ", s) self.failUnlessIn(" (next work in ", s) @@ -3034,7 +3044,7 @@ class BucketCounter(unittest.TestCase, pollmixin.PollMixin): d.addCallback(lambda ignored: self.poll(_watch)) def _check2(ignored): ss.bucket_counter.cpu_slice = orig_cpu_slice - html = w.renderSynchronously() + html = renderSynchronously(w) s = remove_tags(html) self.failUnlessIn("Total buckets: 0 (the number of", s) self.failUnless("Next crawl in 59 minutes" in s or "Next crawl in 60 minutes" in s, s) @@ -3096,20 +3106,20 @@ class BucketCounter(unittest.TestCase, pollmixin.PollMixin): def _check_1(ignored): # no ETA is available yet - html = w.renderSynchronously() + html = renderSynchronously(w) s = remove_tags(html) self.failUnlessIn("complete (next work", s) def _check_2(ignored): # one prefix has finished, so an ETA based upon that elapsed time # should be available. - html = w.renderSynchronously() + html = renderSynchronously(w) s = remove_tags(html) self.failUnlessIn("complete (ETA ", s) def _check_3(ignored): # two prefixes have finished - html = w.renderSynchronously() + html = renderSynchronously(w) s = remove_tags(html) self.failUnlessIn("complete (ETA ", s) d.callback("done") @@ -4064,7 +4074,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): def test_no_server(self): w = StorageStatus(None) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

No Storage Server Running

", html) def test_status(self): @@ -4110,7 +4120,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, "\x00" * 20) ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Accepting new shares: Yes", s) @@ -4130,7 +4140,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, "\x00" * 20) ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Accepting new shares: No", s) @@ -4166,7 +4176,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) @@ -4184,7 +4194,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, "\x00" * 20, readonly_storage=True) ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Accepting new shares: No", s) @@ -4195,7 +4205,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, "\x00" * 20, reserved_space=10e6) ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Reserved space: - 10.00 MB (10000000)", s) @@ -4206,7 +4216,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, "\x00" * 20, reserved_space=10e6) ss.setServiceParent(self.s) w = StorageStatus(ss) - html = w.renderSynchronously() + html = renderSynchronously(w) self.failUnlessIn("

Storage Server Status

", html) s = remove_tags(html) self.failUnlessIn("Reserved space: - 10.00 MB (10000000)", s) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 76a09fda3..e9c2f6bb5 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -6,8 +6,7 @@ from twisted.web.template import ( XMLFile, tags as T, renderer, - renderElement, - flattenString + renderElement ) from allmydata.web.common import ( abbreviate_time, @@ -371,13 +370,6 @@ class StorageStatus(MultiFormatResource): } return json.dumps(d, indent=1) + "\n" - def renderSynchronously(self): - # to appease the test suite. - elem = StorageStatusElement(self.storage, self.nickname) - result = [] - flattenString(None, elem).addCallback(result.append) - return result[0] - # to appease the test suite def renderHTTP(self, ctx=None): """Send HTML or JSON formatted data, based on request. From c061f6830e66ff7770bc0af43eb0442fbdf0f441 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 17:00:54 -0500 Subject: [PATCH 20/41] Use `successResultOf` in `renderSynchronously` Get rid of [].append trick when dealing with the deferred. --- src/allmydata/test/test_storage.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index f37401cc2..0a176bd60 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -2969,9 +2969,8 @@ def renderSynchronously(ss): :param _StorageStatus ss: a StorageStatus instance. """ elem = StorageStatusElement(ss.storage, ss.nickname) - result = [] - flattenString(None, elem).addCallback(result.append) - return result[0] + deferred = flattenString(None, elem) + return unittest.TestCase().successResultOf(deferred) class MyBucketCountingCrawler(BucketCountingCrawler): def finished_prefix(self, cycle, prefix): From aab940f65fb95815c9998b79876adeda5a0fb5f8 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 17:18:37 -0500 Subject: [PATCH 21/41] Remove redundant `None`-checks on `self.storage` When no storage is up, `storage_running()` renderer will return a big honking `no storage server running` message, and no further renderers will be invoked. Therefore the extra defense is probably not required. (I tested this hypothesis. The extra defense is not required, unless there's something I have not seen.) --- src/allmydata/web/storage.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index e9c2f6bb5..cbee2780f 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -42,8 +42,6 @@ class StorageStatusElement(Element): @renderer def nodeid(self, req, tag): - if self.storage is None: - return tag("No storage server running.") return idlib.nodeid_b2a(self.storage.my_nodeid) def _get_storage_stat(self, key): @@ -71,8 +69,6 @@ class StorageStatusElement(Element): :param str key: storage server statistic we want to know. """ - if not self.storage: - return None return self.storage.get_stats().get(key) def render_abbrev_space(self, size): @@ -152,8 +148,6 @@ class StorageStatusElement(Element): @renderer def last_complete_bucket_count(self, req, tag): - if not self.storage: - return tag("No storage server running.") s = self.storage.bucket_counter.get_state() count = s.get("last-complete-bucket-count") if count is None: @@ -162,8 +156,6 @@ class StorageStatusElement(Element): @renderer def count_crawler_status(self, req, tag): - if not self.storage: - return tag("No storage server running.") p = self.storage.bucket_counter.get_progress() return self.format_crawler_progress(p) @@ -200,8 +192,6 @@ class StorageStatusElement(Element): @renderer def lease_expiration_enabled(self, req, tag): - if not self.storage: - return tag("No storage server running.") lc = self.storage.lease_checker if lc.expiration_enabled: return tag("Enabled: expired leases will be removed") @@ -210,8 +200,6 @@ class StorageStatusElement(Element): @renderer def lease_expiration_mode(self, req, tag): - if not self.storage: - return tag("No storage server running.") lc = self.storage.lease_checker if lc.mode == "age": if lc.override_lease_duration is None: @@ -235,16 +223,12 @@ class StorageStatusElement(Element): @renderer def lease_current_cycle_progress(self, req, tag): - if not self.storage: - return tag("No storage server running.") lc = self.storage.lease_checker p = lc.get_progress() return tag(self.format_crawler_progress(p)) @renderer def lease_current_cycle_results(self, req, tag): - if not self.storage: - return tag("No storage server running.") lc = self.storage.lease_checker p = lc.get_progress() if not p["cycle-in-progress"]: @@ -302,8 +286,6 @@ class StorageStatusElement(Element): @renderer def lease_last_cycle_results(self, req, tag): - if not self.storage: - return tag("No storage server running.") lc = self.storage.lease_checker h = lc.get_state()["history"] if not h: From 0382b1ec63cfdcebf5f76f03ee939edbd0901d35 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 17:23:35 -0500 Subject: [PATCH 22/41] Use Unicode strings to render space --- src/allmydata/web/storage.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index cbee2780f..607eeff79 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -73,13 +73,13 @@ class StorageStatusElement(Element): def render_abbrev_space(self, size): if size is None: - return "?" + return u"?" return abbreviate_space(size) def render_space(self, size): if size is None: - return "?" - return "%d" % size + return u"?" + return u"%d" % size @renderer def disk_total(self, req, tag): From 36a486426e970d86fb2dea42ab9ffba0dfc6238c Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 13 Feb 2020 22:02:30 -0500 Subject: [PATCH 23/41] Mark `format_recovered` as static method --- src/allmydata/web/storage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 607eeff79..d670c4231 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -319,7 +319,8 @@ class StorageStatusElement(Element): return tag(p) - def format_recovered(self, sr, a): + @staticmethod + def format_recovered(sr, a): def maybe(d): if d is None: return "?" From 9bb7812148b7aa17cfa641f080ab8e3eb2bce642 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 14 Feb 2020 07:25:49 -0500 Subject: [PATCH 24/41] Add news fragment --- newsfragments/3247.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3247.minor diff --git a/newsfragments/3247.minor b/newsfragments/3247.minor new file mode 100644 index 000000000..e69de29bb From 7625d959bc2735816710af01e80073b102312e17 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 14 Feb 2020 08:26:44 -0500 Subject: [PATCH 25/41] Use to render node nickname and id CI did not like the old way, but it passed in my system. Odd. --- src/allmydata/web/storage_status.xhtml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index 6e0e26008..8f74c478c 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -57,12 +57,12 @@
  • Server Nickname: - +
  • Server Nodeid: - +
  • Accepting new shares: From ff019e5b12d3210046e269519a1e474f1d99ceb3 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 14 Feb 2020 10:10:48 -0500 Subject: [PATCH 26/41] Use BeautifulSoup to check favicon in storage page --- src/allmydata/test/web/test_web.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index f84923521..19753d3b7 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -963,8 +963,9 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi def test_storage(self): d = self.GET("/storage") def _check(res): - self.failUnlessIn('Storage Server Status', res) - self.failUnlessIn(FAVICON_MARKUP, res) + soup = BeautifulSoup(res, 'html5lib') + assert_soup_has_text(self, soup, 'Storage Server Status') + assert_soup_has_favicon(self, soup) res_u = res.decode('utf-8') self.failUnlessIn(u'
  • Server Nickname: fake_nickname \u263A
    • ', res_u) d.addCallback(_check) From 0cbe2871fd80ffd186d54848995c6b6c91ee5ca1 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Fri, 14 Feb 2020 11:42:42 -0500 Subject: [PATCH 27/41] Give the suite the precise string it wants --- src/allmydata/web/storage_status.xhtml | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index 8f74c478c..d052fbabd 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -55,16 +55,8 @@
      -
    • Server Nickname: - - - -
      • -
    • Server Nodeid: - - - -
      • +
    • Server Nickname:
      • +
    • Server Nodeid:
    • Accepting new shares:
    • Total buckets: From 6c3256517a0fa8cd1b8c00267d0f80325f868a70 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 17 Feb 2020 12:52:48 -0500 Subject: [PATCH 28/41] Use slots to render storage stats table --- src/allmydata/web/storage.py | 80 ++++++++------------------ src/allmydata/web/storage_status.xhtml | 26 ++++----- 2 files changed, 36 insertions(+), 70 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index d670c4231..d26f4a5dd 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -82,64 +82,30 @@ class StorageStatusElement(Element): return u"%d" % size @renderer - def disk_total(self, req, tag): - val = self._get_storage_stat("storage_server.disk_total") - return tag(self.render_space(val)) + def storage_stats(self, req, tag): + # Render storage status table that appears near the top of the page. + total = self._get_storage_stat("storage_server.disk_total") + used = self._get_storage_stat("storage_server.disk_used") + free_root = self._get_storage_stat("storage_server.disk_free_for_root") + free_nonroot = self._get_storage_stat("storage_server.disk_free_for_nonroot") + reserved = self._get_storage_stat("storage_server.reserved_space") + available = self._get_storage_stat("storage_server.disk_avail") - @renderer - def disk_total_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.disk_total") - return tag(self.render_abbrev_space(val)) - - @renderer - def disk_used(self, req, tag): - val = self._get_storage_stat("storage_server.disk_used") - return tag(self.render_space(val)) - - @renderer - def disk_used_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.disk_used") - return tag(self.render_abbrev_space(val)) - - @renderer - def disk_free_for_root(self, req, tag): - val = self._get_storage_stat("storage_server.disk_free_for_root") - return tag(self.render_space(val)) - - @renderer - def disk_free_for_root_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.disk_free_for_root") - return tag(self.render_abbrev_space(val)) - - @renderer - def disk_free_for_nonroot(self, req, tag): - val = self._get_storage_stat("storage_server.disk_free_for_nonroot") - return tag(self.render_space(val)) - - @renderer - def disk_free_for_nonroot_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.disk_free_for_nonroot") - return tag(self.render_abbrev_space(val)) - - @renderer - def reserved_space(self, req, tag): - val = self._get_storage_stat("storage_server.reserved_space") - return tag(self.render_space(val)) - - @renderer - def reserved_space_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.reserved_space") - return tag(self.render_abbrev_space(val)) - - @renderer - def disk_avail(self, req, tag): - val = self._get_storage_stat("storage_server.disk_avail") - return tag(self.render_space(val)) - - @renderer - def disk_avail_abbrev(self, req, tag): - val = self._get_storage_stat("storage_server.disk_avail") - return tag(self.render_abbrev_space(val)) + tag.fillSlots( + disk_total = self.render_space(total), + disk_total_abbrev = self.render_abbrev_space(total), + disk_used = self.render_space(used), + disk_used_abbrev = self.render_abbrev_space(used), + disk_free_for_root = self.render_space(free_root), + disk_free_for_root_abbrev = self.render_abbrev_space(free_root), + disk_free_for_nonroot = self.render_space(free_nonroot), + disk_free_for_nonroot_abbrev = self.render_abbrev_space(free_nonroot), + reserved_space = self.render_space(reserved), + reserved_space_abbrev = self.render_abbrev_space(reserved), + disk_avail = self.render_space(available), + disk_avail_abbrev = self.render_abbrev_space(available) + ) + return tag @renderer def accepting_immutable_shares(self, req, tag): diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index d052fbabd..bbf3d2c8a 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -11,15 +11,15 @@

      Storage Server Status

      - +
      - - + + - - + + - - + + - - + + - - + + - - + +
      Total disk space:()()
      Disk space used:- ()- ()
      @@ -28,18 +28,18 @@
      Disk space free (root):()() [see 1]
      Disk space free (non-root):()() [see 2]
      Reserved space:- ()- ()
      @@ -48,8 +48,8 @@
      Space Available to Tahoe:()()
      From 554c477cea5a3c777794923d406137aa1be46f18 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 25 Feb 2020 08:52:45 -0500 Subject: [PATCH 29/41] Prefix member variable with "_" --- src/allmydata/test/test_storage.py | 2 +- src/allmydata/web/storage.py | 40 +++++++++++++++--------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 0a176bd60..56d281688 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -2968,7 +2968,7 @@ def renderSynchronously(ss): """ :param _StorageStatus ss: a StorageStatus instance. """ - elem = StorageStatusElement(ss.storage, ss.nickname) + elem = StorageStatusElement(ss._storage, ss._nickname) deferred = flattenString(None, elem) return unittest.TestCase().successResultOf(deferred) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index d26f4a5dd..9b3daa0cb 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -33,16 +33,16 @@ class StorageStatusElement(Element): :param string nickname: friendly name for storage. """ super(StorageStatusElement, self).__init__() - self.storage = storage - self.nick = nickname + self._storage = storage + self._nickname = nickname @renderer def nickname(self, req, tag): - return self.nick + return self._nickname @renderer def nodeid(self, req, tag): - return idlib.nodeid_b2a(self.storage.my_nodeid) + return idlib.nodeid_b2a(self._storage.my_nodeid) def _get_storage_stat(self, key): """Get storage server statistics. @@ -69,7 +69,7 @@ class StorageStatusElement(Element): :param str key: storage server statistic we want to know. """ - return self.storage.get_stats().get(key) + return self._storage.get_stats().get(key) def render_abbrev_space(self, size): if size is None: @@ -114,7 +114,7 @@ class StorageStatusElement(Element): @renderer def last_complete_bucket_count(self, req, tag): - s = self.storage.bucket_counter.get_state() + s = self._storage.bucket_counter.get_state() count = s.get("last-complete-bucket-count") if count is None: return "Not computed yet" @@ -122,7 +122,7 @@ class StorageStatusElement(Element): @renderer def count_crawler_status(self, req, tag): - p = self.storage.bucket_counter.get_progress() + p = self._storage.bucket_counter.get_progress() return self.format_crawler_progress(p) def format_crawler_progress(self, p): @@ -152,13 +152,13 @@ class StorageStatusElement(Element): @renderer def storage_running(self, req, tag): - if self.storage: + if self._storage: return tag return T.h1("No Storage Server Running") @renderer def lease_expiration_enabled(self, req, tag): - lc = self.storage.lease_checker + lc = self._storage.lease_checker if lc.expiration_enabled: return tag("Enabled: expired leases will be removed") else: @@ -166,7 +166,7 @@ class StorageStatusElement(Element): @renderer def lease_expiration_mode(self, req, tag): - lc = self.storage.lease_checker + lc = self._storage.lease_checker if lc.mode == "age": if lc.override_lease_duration is None: tag("Leases will expire naturally, probably 31 days after " @@ -189,13 +189,13 @@ class StorageStatusElement(Element): @renderer def lease_current_cycle_progress(self, req, tag): - lc = self.storage.lease_checker + lc = self._storage.lease_checker p = lc.get_progress() return tag(self.format_crawler_progress(p)) @renderer def lease_current_cycle_results(self, req, tag): - lc = self.storage.lease_checker + lc = self._storage.lease_checker p = lc.get_progress() if not p["cycle-in-progress"]: return "" @@ -252,7 +252,7 @@ class StorageStatusElement(Element): @renderer def lease_last_cycle_results(self, req, tag): - lc = self.storage.lease_checker + lc = self._storage.lease_checker h = lc.get_state()["history"] if not h: return "" @@ -304,18 +304,18 @@ class StorageStatusElement(Element): class StorageStatus(MultiFormatResource): def __init__(self, storage, nickname=""): super(StorageStatus, self).__init__() - self.storage = storage - self.nickname = nickname + self._storage = storage + self._nickname = nickname def render_HTML(self, req): - return renderElement(req, StorageStatusElement(self.storage, self.nickname)) + return renderElement(req, StorageStatusElement(self._storage, self._nickname)) def render_JSON(self, req): req.setHeader("content-type", "text/plain") - d = {"stats": self.storage.get_stats(), - "bucket-counter": self.storage.bucket_counter.get_state(), - "lease-checker": self.storage.lease_checker.get_state(), - "lease-checker-progress": self.storage.lease_checker.get_progress(), + d = {"stats": self._storage.get_stats(), + "bucket-counter": self._storage.bucket_counter.get_state(), + "lease-checker": self._storage.lease_checker.get_state(), + "lease-checker-progress": self._storage.lease_checker.get_progress(), } return json.dumps(d, indent=1) + "\n" From 110734daf065e7b2706e7d92b2361c21954c11eb Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 5 Mar 2020 15:45:18 -0500 Subject: [PATCH 30/41] Use a helper to exercise render() in storage test cases --- src/allmydata/test/test_storage.py | 50 ++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 56d281688..86b23cd48 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -30,6 +30,7 @@ from allmydata.mutable.layout import MDMFSlotWriteProxy, MDMFSlotReadProxy, \ from allmydata.interfaces import BadWriteEnablerError from allmydata.test.common import LoggingServiceParent, ShouldFailMixin from allmydata.test.common_web import WebRenderingMixin +from nevow.testutil import FakeRequest from allmydata.test.no_network import NoNetworkServer from allmydata.web.storage import ( StorageStatus, @@ -2972,6 +2973,29 @@ def renderSynchronously(ss): deferred = flattenString(None, elem) return unittest.TestCase().successResultOf(deferred) +def renderDeferred(resource, **kwargs): + """ + Use this to exercise an overridden MultiFormatResource.render(), + usually for output=json or render_GET. It returns a Deferred. + + :param _MultiFormatResource resource: an HTTP resource to be rendered. + + """ + # We should be using twisted.web's DummyRequest here instead of + # nevow's FakeRequest, but right now it is a bit of a problem: see + # web/common.py. MultiFormatResource.render() makes a get_arg() + # call, which does a IRequest(ctx_or_req). IRequest can handle + # FakeRequest, but it can't handle DummyRequest. + req = FakeRequest(**kwargs) + req.fields = None + d = defer.maybeDeferred(resource.render, req) + def _done(res): + if isinstance(res, str): + return res + req.v + return req.v + d.addCallback(_done) + return d + class MyBucketCountingCrawler(BucketCountingCrawler): def finished_prefix(self, cycle, prefix): BucketCountingCrawler.finished_prefix(self, cycle, prefix) @@ -3291,7 +3315,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failIfEqual(sr2["configured-diskbytes"], None) self.failIfEqual(sr2["original-sharebytes"], None) d.addCallback(_after_first_bucket) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html_in_cycle(html): s = remove_tags(html) self.failUnlessIn("So far, this cycle has examined " @@ -3366,7 +3390,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnlessEqual(count_leases(mutable_si_2), 1) self.failUnlessEqual(count_leases(mutable_si_3), 2) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("recovered: 0 shares, 0 buckets " @@ -3466,7 +3490,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): d2.addCallback(_after_first_bucket) return d2 d.addCallback(_after_first_bucket) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html_in_cycle(html): s = remove_tags(html) # the first bucket encountered gets deleted, and its prefix @@ -3525,7 +3549,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnless(rec["configured-diskbytes"] >= 0, rec["configured-diskbytes"]) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("Expiration Enabled: expired leases will be removed", s) @@ -3610,7 +3634,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): d2.addCallback(_after_first_bucket) return d2 d.addCallback(_after_first_bucket) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html_in_cycle(html): s = remove_tags(html) # the first bucket encountered gets deleted, and its prefix @@ -3671,7 +3695,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnless(rec["configured-diskbytes"] >= 0, rec["configured-diskbytes"]) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("Expiration Enabled:" @@ -3733,7 +3757,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnlessEqual(count_shares(mutable_si_3), 1) self.failUnlessEqual(count_leases(mutable_si_3), 2) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("The following sharetypes will be expired: immutable.", s) @@ -3790,7 +3814,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): self.failUnlessEqual(count_shares(mutable_si_2), 0) self.failUnlessEqual(count_shares(mutable_si_3), 0) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render1(webstatus)) + d.addCallback(lambda ign: renderDeferred(webstatus)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("The following sharetypes will be expired: mutable.", s) @@ -4021,7 +4045,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): # it also turns all tuples into lists self.failUnlessEqual(corrupt_shares, [[first_b32, 0]]) d.addCallback(_check_json) - d.addCallback(lambda ign: self.render1(w)) + d.addCallback(lambda ign: renderDeferred(w)) def _check_html(html): s = remove_tags(html) self.failUnlessIn("Corrupt shares: SI %s shnum 0" % first_b32, s) @@ -4046,7 +4070,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): corrupt_shares = last["corrupt-shares"] self.failUnlessEqual(corrupt_shares, [[first_b32, 0]]) d.addCallback(_check_json_history) - d.addCallback(lambda ign: self.render1(w)) + d.addCallback(lambda ign: renderDeferred(w)) def _check_html_history(html): s = remove_tags(html) self.failUnlessIn("Corrupt shares: SI %s shnum 0" % first_b32, s) @@ -4060,7 +4084,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): return d def render_json(self, page): - d = self.render1(page, args={"t": ["json"]}) + d = renderDeferred(page, args={"t": ["json"]}) return d class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): @@ -4083,7 +4107,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): ss = StorageServer(basedir, nodeid) ss.setServiceParent(self.s) w = StorageStatus(ss, "nickname") - d = self.render1(w) + d = renderDeferred(w) def _check_html(html): self.failUnlessIn("

      Storage Server Status

      ", html) s = remove_tags(html) @@ -4104,7 +4128,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): return d def render_json(self, page): - d = self.render1(page, args={"t": ["json"]}) + d = renderDeferred(page, args={"t": ["json"]}) return d def test_status_no_disk_stats(self): From f1fe3a75884c2073310727a5f4c4127bfb928645 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 5 Mar 2020 15:46:09 -0500 Subject: [PATCH 31/41] Get rid of WebRenderingMixin in storage test --- src/allmydata/test/test_storage.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 86b23cd48..cff8c3e22 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -29,7 +29,6 @@ from allmydata.mutable.layout import MDMFSlotWriteProxy, MDMFSlotReadProxy, \ SHARE_HASH_CHAIN_SIZE from allmydata.interfaces import BadWriteEnablerError from allmydata.test.common import LoggingServiceParent, ShouldFailMixin -from allmydata.test.common_web import WebRenderingMixin from nevow.testutil import FakeRequest from allmydata.test.no_network import NoNetworkServer from allmydata.web.storage import ( @@ -3185,7 +3184,7 @@ class InstrumentedStorageServer(StorageServer): class No_ST_BLOCKS_StorageServer(StorageServer): LeaseCheckerClass = No_ST_BLOCKS_LeaseCheckingCrawler -class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): +class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): def setUp(self): self.s = service.MultiService() @@ -4087,7 +4086,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): d = renderDeferred(page, args={"t": ["json"]}) return d -class WebStatus(unittest.TestCase, pollmixin.PollMixin, WebRenderingMixin): +class WebStatus(unittest.TestCase, pollmixin.PollMixin): def setUp(self): self.s = service.MultiService() From bae32179bf467e8802d235e5bcd4c1c18411a5c9 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Thu, 5 Mar 2020 15:57:12 -0500 Subject: [PATCH 32/41] Remove StorageStatus.renderHTTP This was added to please the test suite. Pleased to remove it! --- src/allmydata/web/storage.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 9b3daa0cb..63ac47282 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -318,19 +318,3 @@ class StorageStatus(MultiFormatResource): "lease-checker-progress": self._storage.lease_checker.get_progress(), } return json.dumps(d, indent=1) + "\n" - - # to appease the test suite - def renderHTTP(self, ctx=None): - """Send HTML or JSON formatted data, based on request. - - This function contains a bit of nevow-ism, but since this is - only called from the test suite, the nevow-ism should go away - as we update things. - - :param _nevow.context.WovenContext ctx: context is passed on - from the test suite. We get a request out of this - context, and use the request to render a result. - - """ - from nevow.inevow import IRequest - return self.render(IRequest(ctx)) From 9c7357bc61dd74e4b15ae0da2d28a5275acd6b5e Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 30 Mar 2020 16:15:44 -0400 Subject: [PATCH 33/41] Remove an extraneous directive `t:data` is not really a Twisted template directive. Added my mistake, removing now. --- src/allmydata/web/storage_status.xhtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/web/storage_status.xhtml b/src/allmydata/web/storage_status.xhtml index bbf3d2c8a..354f9f177 100644 --- a/src/allmydata/web/storage_status.xhtml +++ b/src/allmydata/web/storage_status.xhtml @@ -57,7 +57,7 @@
      • Server Nickname:
      • Server Nodeid:
        • -
      • Accepting new shares: +
      • Accepting new shares:
      • Total buckets: From 8c92187d9263fa8d092c62c1305200191e850ec5 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 19:00:55 -0400 Subject: [PATCH 34/41] Avoid using nevow FakeRequest in storage test. Use twisted.web.server.Request instead, with a DummyChannel. There's still one line of inevitable nevow now, because of code in web/common.py; but that should be easily replaceable once we switch that over. --- src/allmydata/test/test_storage.py | 47 +++++++++++++++--------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index cff8c3e22..40b9b52e9 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -5,6 +5,16 @@ from twisted.trial import unittest from twisted.internet import defer from twisted.application import service from twisted.web.template import flattenString + +# We need to use `nevow.inevow.IRequest` for now for compatibility +# with the code in web/common.py. Once nevow bits are gone from +# web/common.py, we can use `twisted.web.iweb.IRequest` here. +from nevow.inevow import IRequest + +from twisted.web.server import Request +from twisted.web.test.test_web import DummyChannel +from zope.interface import implements + from foolscap.api import fireEventually import itertools from allmydata import interfaces @@ -29,7 +39,6 @@ from allmydata.mutable.layout import MDMFSlotWriteProxy, MDMFSlotReadProxy, \ SHARE_HASH_CHAIN_SIZE from allmydata.interfaces import BadWriteEnablerError from allmydata.test.common import LoggingServiceParent, ShouldFailMixin -from nevow.testutil import FakeRequest from allmydata.test.no_network import NoNetworkServer from allmydata.web.storage import ( StorageStatus, @@ -2972,28 +2981,20 @@ def renderSynchronously(ss): deferred = flattenString(None, elem) return unittest.TestCase().successResultOf(deferred) -def renderDeferred(resource, **kwargs): - """ - Use this to exercise an overridden MultiFormatResource.render(), - usually for output=json or render_GET. It returns a Deferred. +def renderDeferred(ss): + elem = StorageStatusElement(ss._storage, ss._nickname) + return flattenString(None, elem) - :param _MultiFormatResource resource: an HTTP resource to be rendered. +class JSONRequest(Request): + implements(IRequest) - """ - # We should be using twisted.web's DummyRequest here instead of - # nevow's FakeRequest, but right now it is a bit of a problem: see - # web/common.py. MultiFormatResource.render() makes a get_arg() - # call, which does a IRequest(ctx_or_req). IRequest can handle - # FakeRequest, but it can't handle DummyRequest. - req = FakeRequest(**kwargs) - req.fields = None - d = defer.maybeDeferred(resource.render, req) - def _done(res): - if isinstance(res, str): - return res + req.v - return req.v - d.addCallback(_done) - return d + def __init__(self, **kwargs): + Request.__init__(self, DummyChannel(), **kwargs) + self.args = {"t": ["json"]} + self.fields = {} + +def renderJSON(resource): + return resource.render(JSONRequest()) class MyBucketCountingCrawler(BucketCountingCrawler): def finished_prefix(self, cycle, prefix): @@ -4083,7 +4084,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): return d def render_json(self, page): - d = renderDeferred(page, args={"t": ["json"]}) + d = renderJSON(page) return d class WebStatus(unittest.TestCase, pollmixin.PollMixin): @@ -4127,7 +4128,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin): return d def render_json(self, page): - d = renderDeferred(page, args={"t": ["json"]}) + d = renderJSON(page) return d def test_status_no_disk_stats(self): From 3e7dea7dda3bee3dfb2713297c10f3fac1b2698f Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 19:01:22 -0400 Subject: [PATCH 35/41] Wrap renderer results in tags --- src/allmydata/web/storage.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/allmydata/web/storage.py b/src/allmydata/web/storage.py index 63ac47282..ba6609456 100644 --- a/src/allmydata/web/storage.py +++ b/src/allmydata/web/storage.py @@ -38,11 +38,11 @@ class StorageStatusElement(Element): @renderer def nickname(self, req, tag): - return self._nickname + return tag(self._nickname) @renderer def nodeid(self, req, tag): - return idlib.nodeid_b2a(self._storage.my_nodeid) + return tag(idlib.nodeid_b2a(self._storage.my_nodeid)) def _get_storage_stat(self, key): """Get storage server statistics. @@ -110,20 +110,20 @@ class StorageStatusElement(Element): @renderer def accepting_immutable_shares(self, req, tag): accepting = self._get_storage_stat("storage_server.accepting_immutable_shares") - return {True: "Yes", False: "No"}[bool(accepting)] + return tag({True: "Yes", False: "No"}[bool(accepting)]) @renderer def last_complete_bucket_count(self, req, tag): s = self._storage.bucket_counter.get_state() count = s.get("last-complete-bucket-count") if count is None: - return "Not computed yet" - return str(count) + return tag("Not computed yet") + return tag(str(count)) @renderer def count_crawler_status(self, req, tag): p = self._storage.bucket_counter.get_progress() - return self.format_crawler_progress(p) + return tag(self.format_crawler_progress(p)) def format_crawler_progress(self, p): cycletime = p["estimated-time-per-cycle"] From 8b7ef33b3d3b7caf6a91f6f803111288d2bb5ed4 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 19:05:17 -0400 Subject: [PATCH 36/41] Remove redundant render_json() method --- src/allmydata/test/test_storage.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 40b9b52e9..e20a89da6 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -3399,7 +3399,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): "(2 mutable / 2 immutable),", s) self.failUnlessIn("but expiration was not enabled", s) d.addCallback(_check_html) - d.addCallback(lambda ign: self.render_json(webstatus)) + d.addCallback(lambda ign: renderJSON(webstatus)) def _check_json(raw): data = json.loads(raw) self.failUnlessIn("lease-checker", data) @@ -4036,7 +4036,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): self.failUnlessEqual(so_far["corrupt-shares"], [(first_b32, 0)]) d.addCallback(_after_first_bucket) - d.addCallback(lambda ign: self.render_json(w)) + d.addCallback(lambda ign: renderJSON(w)) def _check_json(raw): data = json.loads(raw) # grr. json turns all dict keys into strings. @@ -4063,7 +4063,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): self.failUnlessEqual(rec["examined-shares"], 3) self.failUnlessEqual(last["corrupt-shares"], [(first_b32, 0)]) d.addCallback(_after_first_cycle) - d.addCallback(lambda ign: self.render_json(w)) + d.addCallback(lambda ign: renderJSON(w)) def _check_json_history(raw): data = json.loads(raw) last = data["lease-checker"]["history"]["0"] @@ -4083,9 +4083,6 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin): d.addBoth(_cleanup) return d - def render_json(self, page): - d = renderJSON(page) - return d class WebStatus(unittest.TestCase, pollmixin.PollMixin): @@ -4116,7 +4113,7 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin): self.failUnlessIn("Accepting new shares: Yes", s) self.failUnlessIn("Reserved space: - 0 B (0)", s) d.addCallback(_check_html) - d.addCallback(lambda ign: self.render_json(w)) + d.addCallback(lambda ign: renderJSON(w)) def _check_json(raw): data = json.loads(raw) s = data["stats"] @@ -4127,9 +4124,6 @@ class WebStatus(unittest.TestCase, pollmixin.PollMixin): d.addCallback(_check_json) return d - def render_json(self, page): - d = renderJSON(page) - return d def test_status_no_disk_stats(self): def call_get_disk_stats(whichdir, reserved_space=0): From 72b8f720802f91d5aef25643b326d1ba98b54cc2 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 19:13:08 -0400 Subject: [PATCH 37/41] Add docstrings to storage test helpers --- src/allmydata/test/test_storage.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index e20a89da6..b69713df5 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -2975,6 +2975,8 @@ def remove_tags(s): def renderSynchronously(ss): """ + Return fully rendered HTML document. + :param _StorageStatus ss: a StorageStatus instance. """ elem = StorageStatusElement(ss._storage, ss._nickname) @@ -2982,10 +2984,20 @@ def renderSynchronously(ss): return unittest.TestCase().successResultOf(deferred) def renderDeferred(ss): + """ + Return a `Deferred` HTML renderer. + + :param _StorageStatus ss: a StorageStatus instance. + """ elem = StorageStatusElement(ss._storage, ss._nickname) return flattenString(None, elem) class JSONRequest(Request): + """ + A Request with t=json argument added to it. + + This is useful to invoke a Resouce.render_JSON() method. + """ implements(IRequest) def __init__(self, **kwargs): @@ -2994,6 +3006,11 @@ class JSONRequest(Request): self.fields = {} def renderJSON(resource): + """Exercise resouce.render_JSON() + + :param _MultiFormatResource resouce: A `twisted.web.resouce.Resource` + that contains a render_JSON() method. + """ return resource.render(JSONRequest()) class MyBucketCountingCrawler(BucketCountingCrawler): From b2b706198042fa39ddc4959d3804647340c8bb24 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 19:18:15 -0400 Subject: [PATCH 38/41] Refactor storage test helpers Rewrite `renderSynchronously()` to use `renderDeferred()` --- src/allmydata/test/test_storage.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index b69713df5..f50c3b352 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -2979,9 +2979,7 @@ def renderSynchronously(ss): :param _StorageStatus ss: a StorageStatus instance. """ - elem = StorageStatusElement(ss._storage, ss._nickname) - deferred = flattenString(None, elem) - return unittest.TestCase().successResultOf(deferred) + return unittest.TestCase().successResultOf(renderDeferred(ss)) def renderDeferred(ss): """ From 82cd5a87fe9582dd1de8e021b8b8a8459d3ba499 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 6 Apr 2020 21:48:49 -0400 Subject: [PATCH 39/41] Use DummyRequest in storage tests Using twisted.web.server.Request causes test_new_style_classes to fail like so: Traceback (most recent call last): Failure: testtools.testresult.real._StringException: Traceback (most recent call last): File ".tox/coverage/lib/python2.7/site-packages/allmydata/test/test_python2_regressions.py", line 69, in test_new_style_classes "Expected to find no classic classes.", File ".tox/coverage/lib/python2.7/site-packages/testtools/testcase.py", line 502, in assertThat raise mismatch_error testtools.matchers._impl.MismatchError: !=: reference = set([]) actual = set([]) : Expected to find no classic classes. Seems that `DummyRequest` is an acceptable new style class. --- src/allmydata/test/test_storage.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index f50c3b352..a0816a6ce 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -11,8 +11,8 @@ from twisted.web.template import flattenString # web/common.py, we can use `twisted.web.iweb.IRequest` here. from nevow.inevow import IRequest -from twisted.web.server import Request -from twisted.web.test.test_web import DummyChannel +# from twisted.web.server import Request +from twisted.web.test.test_web import DummyRequest from zope.interface import implements from foolscap.api import fireEventually @@ -2990,7 +2990,7 @@ def renderDeferred(ss): elem = StorageStatusElement(ss._storage, ss._nickname) return flattenString(None, elem) -class JSONRequest(Request): +class JSONRequest(DummyRequest): """ A Request with t=json argument added to it. @@ -2999,7 +2999,7 @@ class JSONRequest(Request): implements(IRequest) def __init__(self, **kwargs): - Request.__init__(self, DummyChannel(), **kwargs) + DummyRequest.__init__(self, b"/", **kwargs) self.args = {"t": ["json"]} self.fields = {} From 201c08dbe5d0e7c585329261e24082670d999f88 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Mon, 20 Apr 2020 14:35:32 -0400 Subject: [PATCH 40/41] Declare JSON request interface using @implementer "zope.interface.implements(IRequest)" is deprectated in favor of "@zope.interface.implementer(IRequest)" decorator. --- src/allmydata/test/test_storage.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index a0816a6ce..23920a27a 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -11,9 +11,8 @@ from twisted.web.template import flattenString # web/common.py, we can use `twisted.web.iweb.IRequest` here. from nevow.inevow import IRequest -# from twisted.web.server import Request -from twisted.web.test.test_web import DummyRequest -from zope.interface import implements +from twisted.web.test.requesthelper import DummyRequest +from zope.interface import implementer from foolscap.api import fireEventually import itertools @@ -2990,14 +2989,13 @@ def renderDeferred(ss): elem = StorageStatusElement(ss._storage, ss._nickname) return flattenString(None, elem) +@implementer(IRequest) class JSONRequest(DummyRequest): """ A Request with t=json argument added to it. This is useful to invoke a Resouce.render_JSON() method. """ - implements(IRequest) - def __init__(self, **kwargs): DummyRequest.__init__(self, b"/", **kwargs) self.args = {"t": ["json"]} From b3feaae644de727bf564a50df83e1ff4b733f76a Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 21 Apr 2020 10:29:18 -0400 Subject: [PATCH 41/41] Use an inner JSONRequest class with renderJSON Once nevow is removed from web/common.py, we can simplify renderJSON(), like so: def renderJSON(): req = Request() req.args = {"t": ["json"]} req.fields = {} return resource.render(req) But for now we have to live with an inner class that implements the nevow.inevow.IRequest interface. --- src/allmydata/test/test_storage.py | 31 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index 23920a27a..6c5f6e937 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -11,7 +11,8 @@ from twisted.web.template import flattenString # web/common.py, we can use `twisted.web.iweb.IRequest` here. from nevow.inevow import IRequest -from twisted.web.test.requesthelper import DummyRequest +from twisted.web.server import Request +from twisted.web.test.requesthelper import DummyChannel from zope.interface import implementer from foolscap.api import fireEventually @@ -2989,24 +2990,20 @@ def renderDeferred(ss): elem = StorageStatusElement(ss._storage, ss._nickname) return flattenString(None, elem) -@implementer(IRequest) -class JSONRequest(DummyRequest): - """ - A Request with t=json argument added to it. - - This is useful to invoke a Resouce.render_JSON() method. - """ - def __init__(self, **kwargs): - DummyRequest.__init__(self, b"/", **kwargs) - self.args = {"t": ["json"]} - self.fields = {} - def renderJSON(resource): - """Exercise resouce.render_JSON() + """Render a JSON from the given resource.""" + + @implementer(IRequest) + class JSONRequest(Request): + """ + A Request with t=json argument added to it. This is useful to + invoke a Resouce.render_JSON() method. + """ + def __init__(self): + Request.__init__(self, DummyChannel()) + self.args = {"t": ["json"]} + self.fields = {} - :param _MultiFormatResource resouce: A `twisted.web.resouce.Resource` - that contains a render_JSON() method. - """ return resource.render(JSONRequest()) class MyBucketCountingCrawler(BucketCountingCrawler):