Skip to content
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

Disambiguate env vars for model download from Github-reserved values #782

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chuttam
Copy link

@chuttam chuttam commented Aug 25, 2022

Disambiguate env vars for model download from Github-reserved values

  • I read contributing guideline
  • I didn't find a similar pull request already open.
  • My PR is related to Spleeter only, not a derivative product (such as Webapplication, or GUI provided by others)

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 to deezer/spleeter as expected, rather than attempting to hit a URL at my-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.

  • I implemented unit test which ran successfully using poetry run pytest tests/
  • Code has been formatted using poetry run black spleeter
  • Imports has been formatted using `poetry run isort spleeter``

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 deezer#781
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Model download URL points to wrong location when splitting on Github Actions runner
1 participant