fix: support bulk upload when running from docker compose #2940
Conversation
kmahelona
changed the title
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2940 +/- ##
=======================================
Coverage 70.36% 70.37%
=======================================
Files 149 149
Lines 4033 4034 +1
=======================================
+ Hits 2838 2839 +1
Misses 1195 1195
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
kmahelona
changed the title
|
Not sure how to write a test for this, #2940 (comment) |
|
Not sure about the test, maybe ask copilot to make one for you? I found out most linter errors are avpoidable by including the pre-commit hook on your end. Ref: https://libretime.org/docs/contributor-manual/development-workflows/#pre-commit |
| url = os.environ.get( | ||
| "LIBRETIME_GENERAL_PUBLIC_URL", settings.CONFIG.general.public_url | ||
| ) |
There was a problem hiding this comment.
This is not needed, the config already handle env vars by itself.
There was a problem hiding this comment.
in my testing bulk import tries to call http://localhost:8000/api etc. but the docker container can't actually access that. It needs to use http://nginx:8080 instead to access the API to run the import.
There was a problem hiding this comment.
settings.CONFIG.general.public_url = http://localhost:8000/ is the default when you spin up the compose environment.
| environment: | ||
| LIBRETIME_GENERAL_PUBLIC_URL: http://nginx:8080 |
There was a problem hiding this comment.
Hmm this feels risky, could we pass the env vars only when we run the bulk import ?
There was a problem hiding this comment.
The suggestion below makes sense rather than changing the compose. The documentation helps ensure that the right URL is set.
| See the command usage to get available options. | ||
| If you're running from a docker environment, use | ||
| ```bash | ||
| docker-compose run --rm api libretime-api bulk_import --path PATH_THE_DIRECTORY_TO_SCAN |
There was a problem hiding this comment.
| docker-compose run --rm api libretime-api bulk_import --path PATH_THE_DIRECTORY_TO_SCAN | |
| docker-compose run --rm -e LIBRETIME_GENERAL_PUBLIC_URL=http://nginx:8080 api libretime-api bulk_import --path PATH_THE_DIRECTORY_TO_SCAN |
Ideally I would move the import process to python, but this might do the trick for now.
Description
Bulk upload doesn't work when running in a docker-compose environment if the public_url is set to localhost. The fix here sets the api url to
LIBRETIME_GENERAL_PUBLIC_URLenvironment variable if it exists. If it doesn't, then it defaults to thesettings.CONFIG.genera.public_url.LIBRETIME_GENERAL_PUBLIC_URLis only set in docker compose (from what I can tell via search).This is a new feature:
No
I have updated the documentation to reflect these changes:
The documentation, docs/admin-manual/library.md, was updated to show the command for running bulk upload from docker,
Testing Notes
From main do,
The command should fail with an connection error.
How you can replicate my testing:
Checkout this PR, then run
The command should run successfully if the given path has audio files.
Links
https://matrix.to/#/!mkAnQsLsibojOwmqiw:one.ems.host/$vaVktWMuUVUmLdSmXDNbPTKg0ho09V_s2BdR6Uu9wd4?via=one.ems.host&via=matrix.org&via=freie-radios.de