Replace download_url with /file endpoint#478
Open
dima-aignostics wants to merge 2 commits intomainfrom
Open
Conversation
dima-aignostics
requested a review
from helmut-hoffer-von-ankershoffen
as a code owner
|
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK’s artifact download flow to stop relying on the deprecated download_url field and instead obtain fresh presigned URLs via the Platform’s /v1/runs/{run_id}/artifacts/{artifact_id}/file endpoint. It also regenerates the OpenAPI client to the newer API version and adds unit tests around the new download behavior.
Changes:
- Add
Run.get_artifact_download_url()and use it when downloading output artifacts (replacingartifact.download_urlusage). - Update application download helpers to fetch a fresh presigned URL per artifact before downloading.
- Regenerate OpenAPI spec + generated client/docs (new endpoint, scheduling fields, model updates, API version bump).
Reviewed changes
Copilot reviewed 4 out of 48 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/aignostics/application/download_test.py | Adds unit tests for download_item_artifact using the new presigned-URL fetching flow. |
| src/aignostics/platform/resources/runs.py | Adds get_artifact_download_url() and switches artifact download logic to use /file endpoint-derived URLs. |
| src/aignostics/application/_download.py | Updates artifact downloading to request a fresh presigned URL via Run.get_artifact_download_url(). |
| codegen/out/docs/PublicApi.md | Documents new /file endpoint and updated API parameters. |
| codegen/out/aignx/codegen/rest.py | Updates generated client metadata to API version 1.3.0+dev.... |
| codegen/out/aignx/codegen/models/version_read_response.py | Regenerated model (adds regex validator and typing changes). |
| codegen/out/aignx/codegen/models/validation_error_loc_inner.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/validation_error.py | Regenerated model (adds input/ctx fields). |
| codegen/out/aignx/codegen/models/user_read_response.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/scheduling_response.py | New generated model for run scheduling fields in responses. |
| codegen/out/aignx/codegen/models/scheduling_request.py | New generated model for run scheduling constraints in requests. |
| codegen/out/aignx/codegen/models/run_termination_reason.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/run_state.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/run_read_response.py | Regenerated model (adds scheduling). |
| codegen/out/aignx/codegen/models/run_output.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/run_item_statistics.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/run_creation_response.py | Regenerated model (makes run_id required). |
| codegen/out/aignx/codegen/models/run_creation_request.py | Regenerated model (adds scheduling). |
| codegen/out/aignx/codegen/models/output_artifact_visibility.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/output_artifact_scope.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/output_artifact_result_read_response.py | Regenerated model (marks download_url deprecated, adjusts fields). |
| codegen/out/aignx/codegen/models/output_artifact.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/organization_read_response.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/me_read_response.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/item_termination_reason.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/item_state.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/item_result_read_response.py | Regenerated model (adds error_code field ordering/structure changes). |
| codegen/out/aignx/codegen/models/item_output.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/item_creation_request.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/input_artifact_creation_request.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/input_artifact.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/http_validation_error.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/custom_metadata_update_response.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/custom_metadata_update_request.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/artifact_termination_reason.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/artifact_state.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/artifact_output.py | Regenerated header/version update (enum values aligned to new API). |
| codegen/out/aignx/codegen/models/application_version.py | Regenerated model (adds regex validator and typing changes). |
| codegen/out/aignx/codegen/models/application_read_short_response.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/application_read_response.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/models/init.py | Regenerated exports list to include new/updated models. |
| codegen/out/aignx/codegen/exceptions.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/configuration.py | Regenerated header/version update + debug report API version string. |
| codegen/out/aignx/codegen/api_client.py | Regenerated header/version update. |
| codegen/out/aignx/codegen/api/public_api.py | Regenerated API client (adds /file endpoint, updates paths/params). |
| codegen/out/.openapi-generator/FILES | Updates generated file manifest with new scheduling models. |
| codegen/in/openapi.json | Updates input OpenAPI spec (new endpoint, scheduling, deprecations, version bump). |
| codegen/in/archive/openapi_1.3.0+dev.305920c97bb.json | Adds archived OpenAPI snapshot for the new API version. |
Comment on lines
+306
to
+311
| response = requests.get( | ||
| url, | ||
| headers=dict(header_params), | ||
| allow_redirects=False, | ||
| timeout=settings().run_timeout, | ||
| ) |
Comment on lines
+312
to
+323
| if response.status_code == requests.codes.temporary_redirect: | ||
| location = response.headers.get("Location") | ||
| if not location: | ||
| msg = f"307 redirect received but Location header is absent for artifact {artifact_id!r}" | ||
| raise RuntimeError(msg) | ||
| return location | ||
| response.raise_for_status() | ||
| msg = ( | ||
| f"Unexpected status {response.status_code} from artifact URL endpoint " | ||
| f"for artifact {artifact_id!r}; expected 307 redirect" | ||
| ) | ||
| raise RuntimeError(msg) |
Author
There was a problem hiding this comment.
The actual endpoint never returns 200
Comment on lines
+446
to
+447
| with patch("aignostics.application._utils.get_file_extension_for_artifact", return_value=".tiff"): | ||
| download_item_artifact(progress, mock_run, artifact, tmp_path) |
Comment on lines
+483
to
+484
| with patch("aignostics.application._utils.get_file_extension_for_artifact", return_value=".tiff"): | ||
| existing_file = tmp_path / "result.tiff" |
dima-aignostics
force-pushed
the
feat/PAPI-4760-stop-using-download_url
branch
from
c9d8ba5 to
cd1c406
Compare
|
There was a problem hiding this comment.
Pull request overview
This PR updates artifact downloading to stop relying on the deprecated download_url field and instead fetch fresh presigned URLs via the new GET /v1/runs/{run_id}/artifacts/{artifact_id}/file endpoint (capturing the 307 Location redirect). It also refreshes the generated OpenAPI client/spec to include the new endpoint and related schema changes.
Changes:
- Add
Run.get_artifact_download_url()and use it for artifact downloads (platform + application download flows). - Update unit tests to cover the new redirect-based URL retrieval.
- Regenerate OpenAPI artifacts/spec (new endpoint, new scheduling models, schema tweaks, base-path fixes).
Reviewed changes
Copilot reviewed 6 out of 50 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/aignostics/platform/resources/runs_test.py | Adds unit tests for redirect-based presigned URL retrieval via the new /file endpoint. |
| tests/aignostics/application/download_test.py | Import formatting only (no functional change). |
| src/aignostics/platform/resources/runs.py | Implements get_artifact_download_url() and switches run artifact downloading to use it. |
| src/aignostics/application/_download.py | Switches per-artifact download flow to request fresh presigned URLs from the run before downloading. |
| codegen/out/docs/PublicApi.md | Documents the new artifact URL endpoint and updated list-runs query param docs. |
| codegen/out/aignx/codegen/rest.py | Bumps embedded OpenAPI document version string (generated). |
| codegen/out/aignx/codegen/models/version_read_response.py | Generated semver regex validation for version_number. |
| codegen/out/aignx/codegen/models/validation_error_loc_inner.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/validation_error.py | Generated additions for input and ctx fields. |
| codegen/out/aignx/codegen/models/user_read_response.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/scheduling_response.py | New generated model for scheduling fields in run responses. |
| codegen/out/aignx/codegen/models/scheduling_request.py | New generated model for scheduling constraints in run creation. |
| codegen/out/aignx/codegen/models/run_termination_reason.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/run_state.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/run_read_response.py | Adds generated scheduling field support in run responses. |
| codegen/out/aignx/codegen/models/run_output.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/run_item_statistics.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/run_creation_response.py | Makes run_id required in generated response model. |
| codegen/out/aignx/codegen/models/run_creation_request.py | Adds generated scheduling field for run creation payloads. |
| codegen/out/aignx/codegen/models/output_artifact_visibility.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/output_artifact_scope.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/output_artifact_result_read_response.py | Marks download_url deprecated in schema + adjusts ordering/optionality. |
| codegen/out/aignx/codegen/models/output_artifact.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/organization_read_response.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/me_read_response.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/item_termination_reason.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/item_state.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/item_result_read_response.py | Adds generated error_code field and adjusts schema serialization. |
| codegen/out/aignx/codegen/models/item_output.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/item_creation_request.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/input_artifact_creation_request.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/input_artifact.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/http_validation_error.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/custom_metadata_update_response.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/custom_metadata_update_request.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/artifact_termination_reason.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/artifact_state.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/artifact_output.py | OpenAPI version string update (generated, includes new output statuses). |
| codegen/out/aignx/codegen/models/application_version.py | Generated semver regex validation for application version numbers. |
| codegen/out/aignx/codegen/models/application_read_short_response.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/application_read_response.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/models/init.py | Reorders/extends generated model exports (adds scheduling models). |
| codegen/out/aignx/codegen/exceptions.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/configuration.py | Updates debug report API version string (generated). |
| codegen/out/aignx/codegen/api_client.py | OpenAPI version string update (generated). |
| codegen/out/aignx/codegen/api/public_api.py | Adds new generated endpoint + fixes resource paths to avoid double /api prefix. |
| codegen/out/.openapi-generator/FILES | Adds scheduling model files to the generator manifest. |
| codegen/in/openapi.json | Updates source OpenAPI spec: new /file endpoint, deprecations, new fields, auth URLs. |
| codegen/in/archive/openapi_1.3.0+dev.305920c97bb.json | Adds archived OpenAPI snapshot for the regenerated version. |
| CHANGELOG.md | Removes trailing blank lines at end of file. |
Comment on lines
+741
to
+746
| mock_get.assert_called_once_with( | ||
| "https://api.example.com/v1/runs/test-run-id/artifacts/art-1/file", | ||
| headers={}, | ||
| allow_redirects=False, | ||
| timeout=mock_get.call_args[1]["timeout"], | ||
| ) |
Comment on lines
+749
to
+778
| @pytest.mark.parametrize( | ||
| ("status_code", "expected_message"), | ||
| [ | ||
| (200, "Unexpected status 200 from artifact URL endpoint"), | ||
| (307, "307 redirect received but Location header is absent"), | ||
| (404, "Unexpected status 404 from artifact URL endpoint for artifact 'art-1'; expected 307 redirect"), | ||
| ], | ||
| ) | ||
| @pytest.mark.unit | ||
| def test_get_artifact_download_url_errors(app_run, mock_serialize, status_code, expected_message) -> None: | ||
| """Test that get_artifact_download_url raises RuntimeError after unexpected result. | ||
|
|
||
| Args: | ||
| app_run: Run instance with mock API. | ||
| mock_serialize: Mock serializer configured on the API. | ||
| status_code: The HTTP status code to simulate in the response. | ||
| expected_message: The expected error message to be included in the RuntimeError. | ||
| """ | ||
| # Arrange | ||
| mock_response = MagicMock() | ||
| mock_response.__enter__ = Mock(return_value=mock_response) | ||
| mock_response.__exit__ = Mock(return_value=False) | ||
| mock_response.status_code = status_code | ||
| mock_response.headers = {} # No Location header | ||
|
|
||
| with ( | ||
| patch("aignostics.platform.resources.runs.requests.get", return_value=mock_response), | ||
| pytest.raises(RuntimeError, match=expected_message), | ||
| ): | ||
| app_run.get_artifact_download_url("art-1") |
Comment on lines
445
to
456
| if file_path.exists(): | ||
| file_checksum = calculate_file_crc32c(file_path) | ||
| if file_checksum != checksum: | ||
| logger.trace("Resume download for {} to {}", artifact.name, file_path) | ||
| print(f"> Resume download for {artifact.name} to {file_path}") if print_status else None | ||
| else: | ||
| continue | ||
| else: | ||
| downloaded_at_least_one_artifact = True | ||
| logger.trace("Download for {} to {}", artifact.name, file_path) | ||
| print(f"> Download for {artifact.name} to {file_path}") if print_status else None | ||
|
|
Comment on lines
401
to
+405
| msg = f"Download operation failed unexpectedly for run {self.run_id}: {e}" | ||
| raise RuntimeError(msg) from e | ||
|
|
||
| @staticmethod | ||
| def ensure_artifacts_downloaded( | ||
| self, |
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.
No description provided.