Conversation
There was a problem hiding this comment.
Pull request overview
This pull request migrates static image assets from external object storage (hc-cdn.hel1.your-objectstorage.com) to self-hosted files served directly from the application. The change adds a /public static file endpoint and updates references to use the new self-hosted URLs.
Changes:
- Added static file serving via Starlette's StaticFiles middleware
- Introduced
HOSTNAMEenvironment variable for constructing asset URLs - Migrated loading.gif and binoculars.png to local storage under nephthys/public/
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| nephthys/views/home/loading.py | Updated loading.gif URL to use self-hosted path via env.hostname |
| nephthys/views/home/components/ticket_status_pie.py | Updated fallback binoculars.png URL to self-hosted path; added send_heartbeat call |
| nephthys/utils/starlette.py | Added static file serving mount point at /public with StaticFiles middleware |
| nephthys/utils/env.py | Added HOSTNAME environment variable with default value |
| nephthys/public/loading.gif | Added self-hosted loading animation GIF file |
| nephthys/public/binoculars.png | Added self-hosted binoculars fallback image |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| url = "https://hc-cdn.hel1.your-objectstorage.com/s/v3/888f292372d8450449b41dd18767812c72518449_binoculars.png" | ||
| url = f"{env.hostname}/public/binoculars.png" | ||
| caption = "looks like heidi's scrounging around for tickets in the trash" | ||
|
|
There was a problem hiding this comment.
The send_heartbeat function is being called with a URL instead of a descriptive message. This function is meant to send diagnostic heartbeat messages to a Slack channel, not URLs. The URL should not be passed to send_heartbeat - it appears this was meant to be a debugging statement or should be removed entirely. Consider removing this call or replacing it with a proper diagnostic message like "Failed to upload pie chart, using fallback image".
| { | ||
| "type": "image", | ||
| "image_url": "https://hc-cdn.hel1.your-objectstorage.com/s/v3/1c1fc5fb03b8bf46c6ab047c97f962ed930616f0_loading-hugs.gif", | ||
| "image_url": f"{env.hostname}/public/loading.gif", |
There was a problem hiding this comment.
The URL construction f"{env.hostname}/public/loading.gif" assumes that env.hostname does not have a trailing slash. If someone configures HOSTNAME with a trailing slash (e.g., "https://nephthys.hackclub.com/"), this will result in a double slash in the URL ("https://nephthys.hackclub.com//public/loading.gif"). Consider using env.hostname.rstrip('/') or documenting that HOSTNAME should not have a trailing slash.
| "image_url": f"{env.hostname}/public/loading.gif", | |
| "image_url": f"{env.hostname.rstrip('/')}/public/loading.gif", |
| return RedirectResponse(url="https://github.com/hackclub/nephthys") | ||
|
|
||
|
|
||
| STATIC_DIR = Path(Path.cwd() / "nephthys" / "public") |
There was a problem hiding this comment.
Using Path.cwd() for the static directory path could be fragile if the application is run from a different working directory than expected. Consider using Path(__file__).parent.parent / "public" or using a more reliable path resolution method that's relative to the module location rather than the current working directory. This ensures the static files are found regardless of where the application is started from.
| STATIC_DIR = Path(Path.cwd() / "nephthys" / "public") | |
| STATIC_DIR = Path(__file__).resolve().parent.parent / "public" |
|
|
||
| if not url: | ||
| url = "https://hc-cdn.hel1.your-objectstorage.com/s/v3/888f292372d8450449b41dd18767812c72518449_binoculars.png" | ||
| url = f"{env.hostname}/public/binoculars.png" |
There was a problem hiding this comment.
The URL construction f"{env.hostname}/public/binoculars.png" assumes that env.hostname does not have a trailing slash. If someone configures HOSTNAME with a trailing slash (e.g., "https://nephthys.hackclub.com/"), this will result in a double slash in the URL. Consider using env.hostname.rstrip('/') or documenting that HOSTNAME should not have a trailing slash.
| url = f"{env.hostname}/public/binoculars.png" | |
| url = f"{env.hostname.rstrip('/')}/public/binoculars.png" |
SLACK_HELP_CHANNELto .env.sample?threadto ask to use Slack threads?threadexplanationdb.ticket.count()(Improve performance of pie chart generation by usingdb.ticket.count()#62)perf_timer()utility for timing blocks of code (Add aperf_timer()utility for timing blocks of code #108)