From 78f43b3d3f8ff36b60595762d4c2c71aed69001c Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Mon, 29 Nov 2021 14:19:56 -0500
Subject: [PATCH] use integer number of seconds to represent duration
avoids ambiguities and implementation problems of iso8601 duration strings
go back to aniso8601 dependency, since we no longer get anything distinctive
from isodate
---
docs/source/configuration.rst | 14 ++++++-----
setup.cfg | 2 +-
src/_zkapauthorizer/config.py | 8 +++---
src/_zkapauthorizer/lease_maintenance.py | 24 ++++++++++++------
src/_zkapauthorizer/model.py | 25 +++++++++++--------
src/_zkapauthorizer/tests/strategies.py | 10 +++-----
.../tests/test_client_resource.py | 2 +-
.../tests/test_lease_maintenance.py | 2 +-
8 files changed, 48 insertions(+), 39 deletions(-)
diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst
index c8ee935..e6dfe43 100644
--- a/docs/source/configuration.rst
+++ b/docs/source/configuration.rst
@@ -73,22 +73,24 @@ This item controls the frequency at which the lease maintenance crawler runs.
The lease maintenance crawler visits all shares and renews their leases if necessary.
The crawler will run at random intervals.
The client will try to make the average (mean) interval between runs equal to this setting.
-The value is an ISO8601 duration string.
+The value is an integer number of seconds.
For example to run on average every 26 days::
[storageclient.plugins.privatestorageio-zkapauthz-v1]
- lease.crawl-interval.mean = P26D
+ lease.crawl-interval.mean = 2246400
+
lease.crawl-interval.range
~~~~~~~~~~~~~~~~~~~~~~~~~~
This item also controls the frequency of lease maintenance crawler runs.
The random intervals between runs have a uniform distribution with this item's value as its range.
-The value is an ISO8601 duration string.
+The value is an integer number of seconds.
For example to make all intervals fall within a 7 day period::
[storageclient.plugins.privatestorageio-zkapauthz-v1]
- lease.crawl-interval.mean = P3DT12H
+ lease.crawl-interval.mean = 302400
+
lease.min-time-remaining
~~~~~~~~~~~~~~~~~~~~~~~~
@@ -96,11 +98,11 @@ lease.min-time-remaining
This item controls the lease renewal behavior of the lease maintenance crawler.
It specifies an amount of time left on a lease.
If the crawler encounters a lease with less time left than this then it will renew the lease.
-The value is an ISO8601 duration string.
+The value is an integer number of seconds.
For example to renew leases on all shares which will expire in less than one week::
[storageclient.plugins.privatestorageio-zkapauthz-v1]
- lease.min-time-remaining = P7D
+ lease.min-time-remaining = 604800
Server
------
diff --git a/setup.cfg b/setup.cfg
index 272d035..ebbd1ac 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -34,7 +34,7 @@ install_requires =
attrs
zope.interface
eliot
- isodate
+ aniso8601
python-challenge-bypass-ristretto
# The pip resolver sometimes finds treq's dependencies first and these are
# incompatible with Tahoe-LAFS'. So duplicate them here (the ones that
diff --git a/src/_zkapauthorizer/config.py b/src/_zkapauthorizer/config.py
index c7ac61b..ee98742 100644
--- a/src/_zkapauthorizer/config.py
+++ b/src/_zkapauthorizer/config.py
@@ -18,8 +18,6 @@ Helpers for reading values from the Tahoe-LAFS configuration.
from datetime import timedelta
-from isodate import parse_duration
-
from .lease_maintenance import LeaseMaintenanceConfig
@@ -79,8 +77,8 @@ def lease_maintenance_from_tahoe_config(node_config):
def _read_duration(cfg, option, default):
"""
- Read an ISO8601 "duration" from the ZKAPAuthorizer section of a Tahoe-LAFS
- config.
+ Read an integer number of seconds from the ZKAPAuthorizer section of a
+ Tahoe-LAFS config.
:param cfg: The Tahoe-LAFS config object to consult.
:param option: The name of the option to read.
@@ -97,4 +95,4 @@ def _read_duration(cfg, option, default):
)
if value_str is None:
return default
- return parse_duration(value_str)
+ return timedelta(seconds=int(value_str))
diff --git a/src/_zkapauthorizer/lease_maintenance.py b/src/_zkapauthorizer/lease_maintenance.py
index 37bd85d..ad90188 100644
--- a/src/_zkapauthorizer/lease_maintenance.py
+++ b/src/_zkapauthorizer/lease_maintenance.py
@@ -29,7 +29,7 @@ from allmydata.util.hashutil import (
file_cancel_secret_hash,
file_renewal_secret_hash,
)
-from isodate import duration_isoformat, parse_datetime, parse_duration
+from aniso8601 import parse_datetime
from twisted.application.service import Service
from twisted.internet.defer import inlineCallbacks, maybeDeferred
from twisted.python.log import err
@@ -456,24 +456,34 @@ class LeaseMaintenanceConfig(object):
def lease_maintenance_config_to_dict(lease_maint_config):
# type: (LeaseMaintenanceConfig) -> Dict[str, str]
return {
- "lease.crawl-interval.mean": duration_isoformat(
+ "lease.crawl-interval.mean": _format_duration(
lease_maint_config.crawl_interval_mean,
),
- "lease.crawl-interval.range": duration_isoformat(
+ "lease.crawl-interval.range": _format_duration(
lease_maint_config.crawl_interval_range,
),
- "lease.min-time-remaining": duration_isoformat(
+ "lease.min-time-remaining": _format_duration(
lease_maint_config.min_lease_remaining,
),
}
+def _format_duration(td):
+ # type: (timedelta) -> str
+ return str(int(td.total_seconds()))
+
+
+def _parse_duration(duration_str):
+ # type: (str) -> timedelta
+ return timedelta(seconds=int(duration_str))
+
+
def lease_maintenance_config_from_dict(d):
# type: (Dict[str, str]) -> LeaseMaintenanceConfig
return LeaseMaintenanceConfig(
- crawl_interval_mean=parse_duration(d["lease.crawl-interval.mean"]),
- crawl_interval_range=parse_duration(d["lease.crawl-interval.range"]),
- min_lease_remaining=parse_duration(d["lease.min-time-remaining"]),
+ crawl_interval_mean=_parse_duration(d["lease.crawl-interval.mean"]),
+ crawl_interval_range=_parse_duration(d["lease.crawl-interval.range"]),
+ min_lease_remaining=_parse_duration(d["lease.min-time-remaining"]),
)
diff --git a/src/_zkapauthorizer/model.py b/src/_zkapauthorizer/model.py
index 7361b91..27e2fe9 100644
--- a/src/_zkapauthorizer/model.py
+++ b/src/_zkapauthorizer/model.py
@@ -24,7 +24,7 @@ from sqlite3 import OperationalError
from sqlite3 import connect as _connect
import attr
-from isodate import parse_datetime as _parse_datetime
+from aniso8601 import parse_datetime as _parse_datetime
from twisted.logger import Logger
from twisted.python.filepath import FilePath
from zope.interface import Interface, implementer
@@ -39,13 +39,16 @@ from .storage_common import (
from .validators import greater_than, has_length, is_base64_encoded
-def parse_datetime(s):
- # type: (str) -> datetime
+def parse_datetime(s, **kw):
"""
- Parse an ISO8601 datetime, even if it uses a space separator instead of a
- T separator.
+ Like ``aniso8601.parse_datetime`` but accept unicode as well.
"""
- return _parse_datetime(s.replace(u" ", u"T"))
+ if isinstance(s, unicode):
+ s = s.encode("utf-8")
+ assert isinstance(s, bytes)
+ if "delimiter" in kw and isinstance(kw["delimiter"], unicode):
+ kw["delimiter"] = kw["delimiter"].encode("utf-8")
+ return _parse_datetime(s, **kw)
class ILeaseMaintenanceObserver(Interface):
@@ -701,9 +704,9 @@ class VoucherStore(object):
return None
[(started, count, finished)] = activity
return LeaseMaintenanceActivity(
- parse_datetime(started),
+ parse_datetime(started, delimiter=u" "),
count,
- parse_datetime(finished),
+ parse_datetime(finished, delimiter=u" "),
)
@@ -1070,11 +1073,11 @@ class Voucher(object):
return Pending(counter=row[3])
if state == u"double-spend":
return DoubleSpend(
- parse_datetime(row[0]),
+ parse_datetime(row[0], delimiter=u" "),
)
if state == u"redeemed":
return Redeemed(
- parse_datetime(row[0]),
+ parse_datetime(row[0], delimiter=u" "),
row[1],
)
raise ValueError("Unknown voucher state {}".format(state))
@@ -1089,7 +1092,7 @@ class Voucher(object):
# value represents a leap second. However, since we also use
# Python to generate the data in the first place, it should never
# represent a leap second... I hope.
- created=parse_datetime(created),
+ created=parse_datetime(created, delimiter=u" "),
state=state_from_row(state, row[4:]),
)
diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py
index 4a61f8c..df0893b 100644
--- a/src/_zkapauthorizer/tests/strategies.py
+++ b/src/_zkapauthorizer/tests/strategies.py
@@ -355,7 +355,7 @@ def interval_means():
Build timedeltas representing the mean time between lease maintenance
runs.
"""
- return floats(
+ return integers(
# It doesn't make sense to have a negative check interval mean.
min_value=0,
# We can't make this value too large or it isn't convertable to a
@@ -363,10 +363,6 @@ def interval_means():
# questionable value.
max_value=60 * 60 * 24 * 365,
).map(
- # By representing the result as a timedelta we avoid the cases where
- # the lower precision of timedelta compared to float drops the whole
- # value (anything between 0 and 1 microsecond). This is just one
- # example of how working with timedeltas is nicer, in general.
lambda s: timedelta(seconds=s),
)
@@ -378,8 +374,8 @@ def lease_maintenance_configurations():
return builds(
LeaseMaintenanceConfig,
interval_means(),
- timedeltas(min_value=timedelta(0)),
- timedeltas(min_value=timedelta(0)),
+ integers(min_value=0, max_value=60 * 60 * 24 * 365).map(timedelta),
+ integers(min_value=0, max_value=60 * 60 * 24 * 365).map(timedelta),
)
diff --git a/src/_zkapauthorizer/tests/test_client_resource.py b/src/_zkapauthorizer/tests/test_client_resource.py
index 4b1663d..954ad66 100644
--- a/src/_zkapauthorizer/tests/test_client_resource.py
+++ b/src/_zkapauthorizer/tests/test_client_resource.py
@@ -26,6 +26,7 @@ from urllib import quote
import attr
from allmydata.client import config_from_string
+from aniso8601 import parse_datetime
from fixtures import TempDir
from hypothesis import given, note
from hypothesis.strategies import (
@@ -43,7 +44,6 @@ from hypothesis.strategies import (
text,
tuples,
)
-from isodate import parse_datetime
from testtools import TestCase
from testtools.content import text_content
from testtools.matchers import (
diff --git a/src/_zkapauthorizer/tests/test_lease_maintenance.py b/src/_zkapauthorizer/tests/test_lease_maintenance.py
index 57a86d9..a5af529 100644
--- a/src/_zkapauthorizer/tests/test_lease_maintenance.py
+++ b/src/_zkapauthorizer/tests/test_lease_maintenance.py
@@ -222,7 +222,7 @@ class LeaseMaintenanceConfigTests(TestCase):
"""
dumped = lease_maintenance_config_to_dict(config)
loaded = lease_maintenance_config_from_dict(dumped)
- self.assertThat(config, Equals(loaded))
+ self.assertThat(loaded, Equals(config))
class LeaseMaintenanceServiceTests(TestCase):
--
GitLab