Better add-on download handling#9858
Open
seanbudd wants to merge 20 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR centralizes add-on downloading and URL validation in the Python validation layer (instead of the GitHub Actions workflow) to improve submitter-facing error reporting, especially for bad URLs/404s (Fixes #3253).
Changes:
- Removed the workflow
curldownload step so downloading happens via the validation scripts. - Added
downloadAndValidateAddonto reuse URL-format validation + download logic. - Updated
createJson.pyandvalidateSubmissionto use the centralized download+validation path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
validation/_validate/validate.py |
Adds shared download+URL validation helper and adjusts download error handling. |
validation/_validate/createJson.py |
Uses the shared helper before loading the manifest to improve error reporting. |
.github/workflows/sendJsonFile.yml |
Removes workflow-level download step in favor of Python-side handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
pre-commit.ci run |
…Handling' into betterAddonDownloadHandling
…Handling' into betterAddonDownloadHandling
…Handling' into betterAddonDownloadHandling
…Handling' into betterAddonDownloadHandling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #3253
The code to initially download the add-on has bad error handling.
That means for bad URLs / 404s, no good communication to the submitter happens.
Recent example: #9821
We can fix this by using our reusable validation code to download the add-on with instead.
However, when using it, if a 404 is raised from
urlopenit causes an exception, which when logged doesn't provide a clear error message. This caused #3253.Unit tests have been added (AI gen) and system tests have been updated.
.github/workflows/sendJsonFile.ymlworkflow, centralizing this logic in the Python validation code instead. (.github/workflows/sendJsonFile.yml.github/workflows/sendJsonFile.ymlL36-L51)downloadAndValidateAddonfunction was added tovalidate.pyto handle both URL validation and downloading the add-on, yielding errors and raising exceptions as needed. (validation/_validate/validate.pyvalidation/_validate/validate.pyR330-R346)createJson.pynow callsdownloadAndValidateAddonand outputs errors if the download or validation fails, improving error handling and reporting. (validation/_validate/createJson.pyvalidation/_validate/createJson.pyR228-R232)validateSubmissionfunction was updated to usedownloadAndValidateAddon, simplifying the code and ensuring consistent validation logic. (validation/_validate/validate.pyvalidation/_validate/validate.pyL334-R363)downloadAddonto yield and raise exceptions with descriptive messages on failure. (validation/_validate/validate.pyvalidation/_validate/validate.pyR87-R91)