Skip to content

Commit

Permalink
fix: remove default argument now from _seconds_until_refresh (#356)
Browse files Browse the repository at this point in the history
Having now as a default argument means it is set once and then never
adjusted. So in subsequent calls to _seconds_until_refresh the duration
will just continue to grow larger and larger as it evaluates agains the same now

The result of this means that duration // 2 block is always being hit and
will continue to grow. It could grow to be greater than 3600 and lead to errors.
  • Loading branch information
jackwotherspoon authored Jul 25, 2024
1 parent 14d6b1c commit 27f31cd
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 9 deletions.
7 changes: 2 additions & 5 deletions google/cloud/alloydb/connector/refresh_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,19 @@
_refresh_buffer: int = 4 * 60 # 4 minutes


def _seconds_until_refresh(
expiration: datetime, now: datetime = datetime.now(timezone.utc)
) -> int:
def _seconds_until_refresh(expiration: datetime) -> int:
"""
Calculates the duration to wait before starting the next refresh.
Usually the duration will be half of the time until certificate
expiration.
Args:
expiration (datetime.datetime): Time of certificate expiration.
now (datetime.datetime): Current time (UTC)
Returns:
int: Time in seconds to wait before performing next refresh.
"""

duration = int((expiration - now).total_seconds())
duration = int((expiration - datetime.now(timezone.utc)).total_seconds())

# if certificate duration is less than 1 hour
if duration < 3600:
Expand Down
22 changes: 18 additions & 4 deletions tests/unit/test_refresh_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
from datetime import timedelta
from datetime import timezone

import pytest

from google.cloud.alloydb.connector.refresh_utils import _seconds_until_refresh


Expand All @@ -24,8 +26,14 @@ def test_seconds_until_refresh_over_1_hour() -> None:
Test _seconds_until_refresh returns proper time in seconds.
If expiration is over 1 hour, should return duration/2.
"""
now = datetime.now()
assert _seconds_until_refresh(now + timedelta(minutes=62), now) == 31 * 60
# using pytest.approx since sometimes can be off by a second
assert (
pytest.approx(
_seconds_until_refresh(datetime.now(timezone.utc) + timedelta(minutes=62)),
1,
)
== 31 * 60
)


def test_seconds_until_refresh_under_1_hour_over_4_mins() -> None:
Expand All @@ -34,8 +42,14 @@ def test_seconds_until_refresh_under_1_hour_over_4_mins() -> None:
If expiration is under 1 hour and over 4 minutes,
should return duration-refresh_buffer (refresh_buffer = 4 minutes).
"""
now = datetime.now(timezone.utc)
assert _seconds_until_refresh(now + timedelta(minutes=5), now) == 60
# using pytest.approx since sometimes can be off by a second
assert (
pytest.approx(
_seconds_until_refresh(datetime.now(timezone.utc) + timedelta(minutes=5)),
1,
)
== 60
)


def test_seconds_until_refresh_under_4_mins() -> None:
Expand Down

0 comments on commit 27f31cd

Please sign in to comment.