-
Notifications
You must be signed in to change notification settings - Fork 12
Implement minor config fixes #523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Implements minor config fixes to standardize environment variable handling and add stricter validation on durations and URLs.
- Unified
os.getenvcalls with explicit defaults and.strip()for consistent input parsing - Added minimum thresholds for reminder intervals and statistics durations
- Extended
VALID_SEND_INTRODUCTION_REMINDERS_VALUESto include"interval"
Comments suppressed due to low confidence (1)
config.py:187
- Validation for DISCORD_LOG_CHANNEL_WEBHOOK_URL now applies unconditionally, causing an error when the environment variable is unset. Consider restoring a guard (e.g.
if raw_discord_log_channel_webhook_url and (...)) so empty values skip validation.
if not validators.url(
config.py
Outdated
| cls._settings["PURCHASE_MEMBERSHIP_URL"] = None | ||
| return | ||
|
|
||
| if "://" not in raw_purchase_membership_url: |
Copilot
AI
May 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The URL prefix-and-validate pattern is duplicated across several _setup_*_url methods. Consider extracting a helper function to DRY this logic and ensure consistency.
Co-authored-by: Copilot <[email protected]> Signed-off-by: Matty Widdop <[email protected]>
…#508) Signed-off-by: Holly <[email protected]> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Matty Widdop <[email protected]>
|
@CarrotManMatt I think the variables are better having defaults of empty strings, just cleaner that way since we treat no value the same as an empty value. With the casting I've also added casts to all variables whereas we didn't have that before. Is there a difference between using cast or just str? |
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
📢 Thoughts on this report? Let us know! |
#516 is suspended pending these changes, just to reduce the CHONK in that pr