Address code review feedback: improve logging, fix redundant check, clarify docs
- Log exceptions at warning level in ConfigService (not debug) - Include exc_info=True for startup seed failure logging - Remove redundant `is_secret is not None` guard - Clarify ★ markers in README environment variables docs Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> Agent-Logs-Url: https://github.com/christianlouis/pop_puller_to_gmail/sessions/ce88d4a8-d8c2-49b3-95a1-30592105a769
This commit is contained in:
@@ -146,7 +146,7 @@ Emails are forwarded to Gmail via SMTP. This is the legacy method and is used as
|
|||||||
| `POP3_ACCOUNT_N_PASSWORD` | Yes | — | POP3 password |
|
| `POP3_ACCOUNT_N_PASSWORD` | Yes | — | POP3 password |
|
||||||
| `POP3_ACCOUNT_N_USE_SSL` | No | `true` | Use SSL/TLS |
|
| `POP3_ACCOUNT_N_USE_SSL` | No | `true` | Use SSL/TLS |
|
||||||
|
|
||||||
#### SMTP Settings ★
|
#### SMTP Settings (★ database-configurable)
|
||||||
|
|
||||||
| Variable | Required | Default | Description |
|
| Variable | Required | Default | Description |
|
||||||
|----------|----------|---------|-------------|
|
|----------|----------|---------|-------------|
|
||||||
@@ -156,7 +156,7 @@ Emails are forwarded to Gmail via SMTP. This is the legacy method and is used as
|
|||||||
| `SMTP_PASSWORD` | For SMTP | — | SMTP password (App Password) |
|
| `SMTP_PASSWORD` | For SMTP | — | SMTP password (App Password) |
|
||||||
| `SMTP_USE_TLS` | No | `true` | Use STARTTLS |
|
| `SMTP_USE_TLS` | No | `true` | Use STARTTLS |
|
||||||
|
|
||||||
#### Gmail API Settings ★
|
#### Gmail API Settings (★ database-configurable)
|
||||||
|
|
||||||
| Variable | Required | Default | Description |
|
| Variable | Required | Default | Description |
|
||||||
|----------|----------|---------|-------------|
|
|----------|----------|---------|-------------|
|
||||||
@@ -164,7 +164,7 @@ Emails are forwarded to Gmail via SMTP. This is the legacy method and is used as
|
|||||||
| `GOOGLE_CLIENT_SECRET` | For Gmail API | — | Google OAuth2 client secret |
|
| `GOOGLE_CLIENT_SECRET` | For Gmail API | — | Google OAuth2 client secret |
|
||||||
| `GMAIL_API_ENABLED` | No | `true` | Enable Gmail API delivery |
|
| `GMAIL_API_ENABLED` | No | `true` | Enable Gmail API delivery |
|
||||||
|
|
||||||
#### Processing Settings ★
|
#### Processing Settings (★ database-configurable)
|
||||||
|
|
||||||
| Variable | Required | Default | Description |
|
| Variable | Required | Default | Description |
|
||||||
|----------|----------|---------|-------------|
|
|----------|----------|---------|-------------|
|
||||||
|
|||||||
+1
-1
@@ -37,7 +37,7 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
|
|||||||
async with async_session_maker() as db:
|
async with async_session_maker() as db:
|
||||||
await ConfigService.seed_defaults(db)
|
await ConfigService.seed_defaults(db)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
logger.warning("Could not seed default settings: %s", exc)
|
logger.warning("Could not seed default settings: %s", exc, exc_info=True)
|
||||||
|
|
||||||
yield
|
yield
|
||||||
# Shutdown
|
# Shutdown
|
||||||
|
|||||||
@@ -190,8 +190,12 @@ class ConfigService:
|
|||||||
setting.value, # type: ignore[arg-type]
|
setting.value, # type: ignore[arg-type]
|
||||||
setting.value_type or "string", # type: ignore[arg-type]
|
setting.value_type or "string", # type: ignore[arg-type]
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception as exc:
|
||||||
logger.debug("DB lookup failed for key=%s, falling back to env", key)
|
logger.warning(
|
||||||
|
"DB lookup failed for key=%s, falling back to env: %s",
|
||||||
|
key,
|
||||||
|
exc,
|
||||||
|
)
|
||||||
|
|
||||||
env_val = os.getenv(key)
|
env_val = os.getenv(key)
|
||||||
if env_val is not None:
|
if env_val is not None:
|
||||||
@@ -216,8 +220,8 @@ class ConfigService:
|
|||||||
setting.value, # type: ignore[arg-type]
|
setting.value, # type: ignore[arg-type]
|
||||||
setting.value_type or "string", # type: ignore[arg-type]
|
setting.value_type or "string", # type: ignore[arg-type]
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception as exc:
|
||||||
logger.debug("DB bulk lookup failed, falling back to env")
|
logger.warning("DB bulk lookup failed, falling back to env: %s", exc)
|
||||||
|
|
||||||
for key in keys:
|
for key in keys:
|
||||||
if key not in result:
|
if key not in result:
|
||||||
@@ -253,8 +257,7 @@ class ConfigService:
|
|||||||
existing.value_type = value_type # type: ignore[assignment]
|
existing.value_type = value_type # type: ignore[assignment]
|
||||||
if description is not None:
|
if description is not None:
|
||||||
existing.description = description # type: ignore[assignment]
|
existing.description = description # type: ignore[assignment]
|
||||||
if is_secret is not None:
|
existing.is_secret = is_secret # type: ignore[assignment]
|
||||||
existing.is_secret = is_secret # type: ignore[assignment]
|
|
||||||
if category is not None:
|
if category is not None:
|
||||||
existing.category = category # type: ignore[assignment]
|
existing.category = category # type: ignore[assignment]
|
||||||
await db.commit()
|
await db.commit()
|
||||||
|
|||||||
Reference in New Issue
Block a user