Support float ttl

[karpetrosyan]

This pull request adds float ttl support for all kinds of storage.

Also, floating ttl support means that now we can use tiny sleeps for storage ttl tests, which improves the tests performance.

For SQLite, I switched the date_created field from datetime to real in order to compare Unix times.
For Redis, I have used the PX option, which can set the timeout using miliseconds instead of seconds.
For FileSystem, no changes were made because st_mtime is a floating number.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4e04a7c) 100.00% compared to head (67c3711) 100.00%.

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #107   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           27        27           
  Lines         1775      1785   +10     
=========================================
+ Hits          1775      1785   +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[parkerhancock]

Hey! Thanks for assigning this one to me for review. I have a gut-check question for you though - do we need this feature? Other than running tests faster, I can't think of any possible use requiring float-level precision for a ttl value. And I'm seeing issues where Redis doesn't support floats in the way that this PR suggests (at least not for all versions). So we'd be taking on quite a bit of complexity for limited advantage.

If the goal is to speed up the tests (which is a totally valid reason), then I think the easier solution would be to build off the BaseClock object and mock out the actual clock. The BaseClock is already an injectable dependency into the Controller object, so would the easier solution here be to make it injectable into the Storages objects? Then you could make something like this:

from ._utils import BaseClock

class MockClock(BaseClock): 
    def __init__(self):
         self.time = 0

    def now(self) -> int: 
         return self.time

    def advance(self, increment: int) -> None:
         self.time += increment

And then in testing, just add a call to clock.advance() rather than using some form of a wait to change the time for calculating ttl.

[karpetrosyan]

The idea that we can inject clocks into the storage objects sounds really good, although I think the current implementation needs to be added, at least because float timeouts are more user-friendly and seem more natural than integer ones.

There are also some complexities in the solution where we should inject clock objects. That is, when we use Redis or SQLite databases, we cannot mock the timeout because it's handled by them.

Here is the code that handles ttl for the redis:

hishel/hishel/_async/_storages.py

Lines 287 to 291 in a6b4c65

	await self._client.set(
	key,
	self._serializer.dumps(response=response, request=request, metadata=metadata),
	ex=self._ttl,
	)

We simply use the ex option to set the timeout, so we don't handle them. We can mock the clock and play with the timeouts when we use the filesystem storage, but for other storage, I don't know how mocking the clock would help.

[parkerhancock]

Understood. I still think we're going to have a problem with using floats as ttl values. Bottom-line, both SQLite and Redis represent datetimes as some form of int.

For SQLite, the date_created field only has second-level precision. As the documentation states, SQLite actually doesn't have an internal datetime storage format, instead it stores datetimes as either TEXT (ISO-8601), REAL (days since the Julian date epoch), or INT (seconds since the Unix Epoch): (https://www.sqlite.org/datatype3.html).

For Redis, the ex field stores an expiry ttl in integer sections, or you can use the px field to specify an expiry ttl in integer milliseconds. (https://redis.io/commands/set/)

For either, the best precision you can get is millisecond precision, so float is still an inappropriate data type since you can have a float much smaller than a millisecond.

So, I think what we need to do is keep TTL an int value, but we could have it represent miliseconds, rather than seconds to live. If we want to keep backward compatibility with the current Hishel implementation, then I think we need to add a new keyword argument to storages for it as ttl_ms, which specifies TTL in milliseconds, and then raise an error if both ttl_ms and ttl are both set.

[karpetrosyan]

Bottom-line, both SQLite and Redis represent datetimes as some form of int.

We can use SQLite REAL to work with the Unix times, as I have done in this PR.

Yes, Redis represents timeouts as integer values, so I have created a simple float_seconds_to_int_milliseconds function that we can always use for the PX option that Redis provides us.

This PR is a huge performance improvement for the tests.
In the master branch, running all the tests takes around 12 seconds, while in this branch, it just takes around 2 seconds.

If we want to keep backward compatibility with the current Hishel implementation, then I think we need to add a new keyword argument to storages for it as ttl_ms, which specifies TTL in milliseconds, and then raise an error if both ttl_ms and ttl are both set.

This PR doesn't break the backward compatability. It just adds a float-type support for ttl options, which I think is a far simpler solution than introducing a new argument special for millisecond ttls.

It's just not going to work as expected if the user has the sqlite cache because this PR has changed the table schema for sqlite tables, so we can mention it in the release notes.

I think it's a good trade-off.

[parkerhancock]

The code looks good, I don't have any further comments on the code itself. I still question the wisdom of using float values for what are ints, and don't love adding features for testing purposes. But I feel like I've been heard, and I'm happy to go along with this compromise.