Fix five code-review findings: token auth, sync rate-limiting, model drift, FK cascade
- garmin_connect_sync: revert to garth.loads() for token auth — login(tokenstore=) dispatches on len>512, treating compact tokens as filesystem paths and forcing a full re-login on every sync. Explicitly set display_name from the embedded profile. - garmin_connect_sync: restore incremental sync for both activities and wellness — always re-fetching the full lookback window was generating ~270 Garmin API calls per wellness sync run, risking rate-limits. Now uses since-1d when since is set. Add 0.25s per-day sleep in sync_wellness as an additional rate-limit guard. - models/user.py: replace the dropped uq_pr_current UniqueConstraint in PersonalRecord.__table_args__ with the partial Index the DB actually has, so the model and live schema no longer permanently diverge. - models/user.py: add ondelete="SET NULL" to Activity.named_route_id FK so the DB cascade handles unlinks if routes are deleted outside the API endpoint. - main.py: add startup migration to re-add activities_named_route_id_fkey with ON DELETE SET NULL on existing deployments. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -47,15 +47,18 @@ def authenticate_garmin(email: str, password_enc: str, token_store: Optional[str
|
||||
import garminconnect
|
||||
|
||||
# Try stored OAuth token first.
|
||||
# Must call login(tokenstore=...) rather than garth.loads() directly so that
|
||||
# garmin.display_name is populated — it's required by get_user_summary() and
|
||||
# several other endpoints. Without it every stats call silently returns None.
|
||||
# Use garth.loads() directly (always treats the argument as an inline string).
|
||||
# garmin.login(tokenstore=...) dispatches on len>512, treating short tokens as
|
||||
# filesystem paths and raising FileNotFoundError on every token-based auth attempt.
|
||||
# After loads(), set display_name from the embedded profile — required by
|
||||
# get_stats(), get_sleep_data(), and other endpoints that build URLs from it.
|
||||
if token_store:
|
||||
try:
|
||||
garmin = garminconnect.Garmin(
|
||||
email=email, password=decrypt_password(password_enc)
|
||||
)
|
||||
garmin.login(tokenstore=token_store)
|
||||
garmin.garth.loads(token_store)
|
||||
garmin.display_name = (garmin.garth.profile or {}).get("displayName", "")
|
||||
return garmin, None
|
||||
except Exception as exc:
|
||||
logger.info("Garmin token invalid (%s), re-authenticating", exc)
|
||||
@@ -76,7 +79,7 @@ def sync_activities(garmin, user_id: int, since: Optional[datetime],
|
||||
|
||||
lookback_days controls the start date on every sync:
|
||||
-1 → full history back to 2010 on first sync, then incremental (since-1d)
|
||||
N → always look back N days from today (dedup prevents re-downloading)
|
||||
N → incremental (since-1d) when since is set; else last N days on first sync
|
||||
Returns the number of new activities queued.
|
||||
"""
|
||||
import time
|
||||
@@ -87,6 +90,9 @@ def sync_activities(garmin, user_id: int, since: Optional[datetime],
|
||||
if lookback_days == -1:
|
||||
# All-time: full pull on first sync, incremental thereafter
|
||||
start_date = (since - timedelta(days=1)).date() if since else date(2010, 1, 1)
|
||||
elif since:
|
||||
# Incremental: one day overlap to catch any late-arriving activities
|
||||
start_date = (since - timedelta(days=1)).date()
|
||||
else:
|
||||
start_date = date.today() - timedelta(days=max(lookback_days, 1))
|
||||
end_date = date.today()
|
||||
@@ -180,18 +186,22 @@ def sync_wellness(garmin, user_id: int, since: Optional[datetime], db,
|
||||
|
||||
lookback_days controls the window on every sync:
|
||||
-1 → full history back to 2010 on first sync, then incremental (since-1d)
|
||||
N → always cover the last N days (upsert is safe to re-run)
|
||||
N → incremental (since-1d) when since is set; else last N days on first sync
|
||||
Returns the number of days upserted.
|
||||
"""
|
||||
from sqlalchemy import text
|
||||
|
||||
if lookback_days == -1:
|
||||
start_date = (since - timedelta(days=1)).date() if since else date(2010, 1, 1)
|
||||
elif since:
|
||||
# Incremental: one day overlap to catch any late-arriving wellness data
|
||||
start_date = (since - timedelta(days=1)).date()
|
||||
else:
|
||||
start_date = date.today() - timedelta(days=max(lookback_days, 1))
|
||||
days = (date.today() - start_date).days + 1
|
||||
processed = 0
|
||||
|
||||
import time as _time
|
||||
for i in range(max(days, 1)):
|
||||
day = start_date + timedelta(days=i)
|
||||
day_str = day.isoformat()
|
||||
@@ -199,6 +209,7 @@ def sync_wellness(garmin, user_id: int, since: Optional[datetime], db,
|
||||
stats = _safe(garmin.get_stats, day_str)
|
||||
sleep_data = _safe(garmin.get_sleep_data, day_str)
|
||||
hrv_data = _safe(garmin.get_hrv_data, day_str)
|
||||
_time.sleep(0.25) # avoid hammering Garmin's wellness API
|
||||
|
||||
row = _parse_day(stats, sleep_data, hrv_data)
|
||||
if not row:
|
||||
|
||||
Reference in New Issue
Block a user