Refactor setup.py to improve GitHub API error handling and configuration#532
Conversation
|
@jrhoads Thanks for getting this up so quickly! I'm a bit wary of the inline env vars, as (I think) they would override anything in .env, so then local stuff gets silently ignored. Kind of non-obvious. I think it also opens us up a bit towards the "oopsie" path of committing real creds (whereas in the setup now .env is gitignored). Could we just keep .env as the source of truth and add a .env.example with the dummy values? Separately, I think swapping the rorapi bind mount for develop.watch means docker compose up will no longer hot-reload? Is that correct? If so, we should probably update the README/docs so folks know to run with docker compose watch. |
|
I don't have strong opinions on the docker compose stuff. I was encountering some friction when running the app with docker compose and this cleared it up. I was not getting the hot reload behavior in passenger and this is why I switched to the develop.watch methods. For Env variable I just looked up the precedence for container runtime:
If I need it I can do this stuff with a local docker-compose.override.yaml file. I can go ahead and revert most of the stuff in docker-compose.yaml. It's not the real focus of this work. |
|
@jrhoads Thanks for doing so. Approved! |
Purpose
This PR refactors the
setup.pymanagement command to improve error handling and configuration management when interacting with the GitHub API to fetch ROR data dumps.Approach
The changes focus on making the
setup.pycommand more robust by:Key Modifications
try-exceptblocks to catch specificrequests.exceptionslikeTimeout,ConnectionError, and generalRequestException.response.raise_for_status()with specific checks for common GitHub API error codes (401, 403, 404) to provide more informative error messages.build_github_headers,get_contents_url) with checks for missing configuration values, raisingCommandErrorif necessary.try-except ValueErrorblock for JSON decoding to handle cases where the GitHub API returns invalid JSON.repo_contentsis a list as expected from the GitHub API.SystemExitwithCommandErrorfor more graceful error reporting within Django management commands.REQUEST_TIMEOUT_SECONDSconstant for controlling request timeouts.Important Technical Details
get_ror_dump_shafunction now explicitly handles various network and HTTP errors from the GitHub API, providing clearer feedback to the user.CommandErrorensures that errors are reported appropriately by Django's management command system.build_github_headersfunction ensures that the GitHub token is present and correctly formatted.Types of changes
Reviewer, please remember our guidelines: