Skip to content

Commit

Permalink
Cover ttl=0 (expire always) _DNSCacheTable case (aio-libs#7014)
Browse files Browse the repository at this point in the history
<!-- Thank you for your contribution! -->

## What do these changes do?
Fix bug when using ttl_dns_cache=0

<!-- Please give a short brief about these changes. -->
When using `ttl_dns_cache=0`, hostname is added to cache and on expire
check there is a KeyError as it is never added into _timestamp
dictionary.
Added a short description to docs ClientSession advising to use
`use_dns_cache` instead of `ttl_dns_cache=0` for always expiring cache.

Alternative to this change is to enforce user to use positive none 0
float or None for `ttl_dns_cache` during init time and in case of using
0 implicitly change `use_dns_cache` to False or raise exception.
## Are there changes in behavior for the user?
No.

<!-- Outline any notable behaviour for the end users. -->

## Related issue number

<!-- Are there any issues opened that will be resolved by merging this
change? -->

## Checklist

- [X] I think the code is well written
- [X] Unit tests for the changes exist
- [X] Documentation reflects the changes
- [X] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt`
  * The format is &lt;Name&gt; &lt;Surname&gt;.
  * Please keep alphabetical order, the file is sorted by names.
- [X] Add a new news fragment into the `CHANGES` folder
  * name it `<issue_id>.<type>` for example (588.bugfix)
* if you don't have an `issue_id` change it to the pr id after creating
the pr
  * ensure type is one of the following:
    * `.feature`: Signifying a new feature.
    * `.bugfix`: Signifying a bug fix.
    * `.doc`: Signifying a documentation improvement.
    * `.removal`: Signifying a deprecation or removal of public API.
* `.misc`: A ticket has been closed, but it is not of interest to users.
* Make sure to use full sentences with correct case and punctuation, for
example: "Fix issue with non-ascii contents in doctest text files."

Co-authored-by: Sam Bull <aa6bs0@sambull.org>
  • Loading branch information
laky55555 and Dreamsorcerer committed Oct 22, 2022
1 parent 1822413 commit 12f56d7
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGES/7014.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed bug when using ``TCPConnector`` with ``ttl_dns_cache=0``.
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ Illia Volochii
Ilya Chichak
Ilya Gruzinov
Ingmar Steen
Ivan Lakovic
Ivan Larin
Jacob Champion
Jaesung Lee
Expand Down
4 changes: 2 additions & 2 deletions aiohttp/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -678,13 +678,13 @@ def __contains__(self, host: object) -> bool:
def add(self, key: Tuple[str, int], addrs: List[Dict[str, Any]]) -> None:
self._addrs_rr[key] = (cycle(addrs), len(addrs))

if self._ttl:
if self._ttl is not None:
self._timestamps[key] = monotonic()

def remove(self, key: Tuple[str, int]) -> None:
self._addrs_rr.pop(key, None)

if self._ttl:
if self._ttl is not None:
self._timestamps.pop(key, None)

def clear(self) -> None:
Expand Down
23 changes: 20 additions & 3 deletions tests/test_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -2179,10 +2179,27 @@ def test_not_expired_ttl(self) -> None:
dns_cache_table.add("localhost", ["127.0.0.1"])
assert not dns_cache_table.expired("localhost")

async def test_expired_ttl(self, loop: Any) -> None:
dns_cache_table = _DNSCacheTable(ttl=0.01)
def test_expired_ttl(self, monkeypatch: pytest.MonkeyPatch) -> None:
dns_cache_table = _DNSCacheTable(ttl=1)
monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 1)
dns_cache_table.add("localhost", ["127.0.0.1"])
await asyncio.sleep(0.02)
monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 2)
assert not dns_cache_table.expired("localhost")
monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 3)
assert dns_cache_table.expired("localhost")

def test_never_expire(self, monkeypatch: pytest.MonkeyPatch) -> None:
dns_cache_table = _DNSCacheTable(ttl=None)
monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 1)
dns_cache_table.add("localhost", ["127.0.0.1"])
monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 10000000)
assert not dns_cache_table.expired("localhost")

def test_always_expire(self, monkeypatch: pytest.MonkeyPatch) -> None:
dns_cache_table = _DNSCacheTable(ttl=0)
monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 1)
dns_cache_table.add("localhost", ["127.0.0.1"])
monkeypatch.setattr("aiohttp.connector.monotonic", lambda: 1.00001)
assert dns_cache_table.expired("localhost")

def test_next_addrs(self, dns_cache_table: Any) -> None:
Expand Down

0 comments on commit 12f56d7

Please sign in to comment.