Disambiguate env vars for model download from Github-reserved values #782
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Disambiguate env vars for model download from Github-reserved values
Description
When running in a Github Action runner,
GITHUB_REPOSITORY
is a reserved environment variable that ideally should not be overridden for fear of erratic/unexpected side-effects.This commit renames the environment variables to be a bit more distinct and related to their purpose within spleeter i.e. for building a model download destination.
Fixes #781
How this patch was tested
Ran spleeter in a Github Action job, without having to override the new
GITHUB_MODEL_REPOSITORY
variable, and it automatically defaulted todeezer/spleeter
as expected, rather than attempting to hit a URL atmy-org/my-repo
.Unit test was NOT added, because the existing test suite runs an
httpx
GET
request, which already fails to being with(attempting to access a public URL from a command line test suite run). Adequate tests for this functionality already do not exist, and would ideally require adding HTTP request/response stubs (perhaps via (pytest-httpx)[https://pypi.org/project/pytest-httpx/]). For the simple nature of this solution, I preferred to not introduce a whole new dependency to the call stack... but happy to do so if deemed necessary for the acceptance of this PR.
poetry run pytest tests/
poetry run black spleeter