Skip to content

OpenConceptLab/ocl_issues#2864 | Export API: new download names, and GET redirects rather than streams - #68

Merged
paynejd merged 3 commits into
mainfrom
docs/2864-export-download-name
Oct 7, 2026
Merged

paynejd merged 3 commits into
mainfrom
docs/2864-export-download-name

Conversation

@paynejd

@paynejd paynejd commented Oct 7, 2026

Copy link
Copy Markdown
Member

Part of OpenConceptLab/ocl_issues#2864 (ocl-docs piece). Merge once OpenConceptLab/oclapi2#929 is deployed, so the page matches what the API returns.

Changes to oclapi/apireference/exportapi.md

  • Filename: replaces [:repositoryId]_[:repoVersion].[:lastUpdated].zip with the name the API now sets, [:ownerType]_[:owner]_[:repoType]_[:repo]_[:repoVersion]_[:expansion]_[:lastUpdated].zip, with rules and examples.
  • GET and HEAD: the endpoint returns 302 Found to a signed download URL. It never streamed the zip. The filename is in the download's Content-Disposition header and in the signed URL's response-content-disposition.
  • HEAD version: the notes said HEAD exports always return 405. Repository admins can create and download them, and everyone else gets 405.

other/subscriptions.md needs no change. It tells clients to read lastUpdated from the filename, and every name still ends with it.

🤖 Generated with Claude Code

…GET redirects rather than streams

Documents the export filename the API now sets,
[:ownerType]_[:owner]_[:repoType]_[:repo]_[:repoVersion]_[:expansion]_[:lastUpdated].zip,
and describes GET/HEAD as a 302 to a signed download URL. Also corrects the
HEAD-version notes: repo admins can export HEAD.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex adversarial review, pass 1 (codex-cli 0.160.1, commit c353d3d)

Reviewed against oclapi2#929 (core/... paths below are in oclapi2).

Ran git diff origin/main...HEAD. No files modified. Reviewed documentation commit c353d3d against oclapi2 commit 7347e6c, including the requested retained sections.

  1. [P2] Processing and missing exports are conflated — exportapi.md:4. GET/HEAD returns 208 when is_exporting is true; 204 means no export exists. The “three possible results” list at line 49 also omits 208 and the explicit 500 when signing fails. See core/common/mixins.py:1056–1075.

  2. [P2] is_processing=False does not establish export availability — exportapi.md:4. Calling this method “still correct” overstates its guarantee: export tasks remove their processing marker in finally, including after failure (core/common/tasks.py:126–137,154–170). Availability requires checking the export endpoint.

  3. [P2] Not every collection export includes an expansion — exportapi.md:23. Collections can have no default expansion, and write_export_file() still produces an export with empty concept/mapping arrays. The filename helper then omits the expansion segment. See core/collections/models.py:359–364, core/common/utils.py:217–228, and core/common/models.py:1416–1418.

  4. [P2] Timestamp provenance is incompletely described — exportapi.md:25. The timestamp normally comes from the selected cached storage key, not the repository’s current last change. get_export_path() can select an older cached export. Also, empty repositories use updated_at through last_child_update, despite having no concept/mapping change. See core/common/models.py:841–846,1398–1422.

  5. [P2] The filename timestamp cannot be passed directly to updatedSince — exportapi.md:35. The newly specified 2026-09-30_123456 format is rejected by from_string_to_date() and becomes None, removing the date filter. Document conversion to 2026-09-30T12:34:56Z, consistent with subscriptions.md. Confirmed with an isolated parser probe; see core/common/utils.py:157–169 and core/common/mixins.py:713–720.

  6. [P2] HEAD-version permissions and rejection codes are oversimplified — exportapi.md:58, repeated at line 107. The gate permits staff, superusers, and is_admin_for(version); that helper includes the owning user or any member of the owning organization. Authentication and object permissions run first, so “anyone else” can receive 401/403, rather than universally 405. See core/common/mixins.py:1030–1054, the export views’ permission classes, and core/users/models.py:175–180.

  7. [P2] HEAD polling needs an explicit instruction to disable redirects — exportapi.md:59. Clients must inspect the API’s initial 302. Automatically following it checks the storage response instead, and both storage adapters generate a URL signed for GET, including when Django dispatches HEAD through get(). See aws.py:68–72 and minio.py:89–102.

  8. [P2] POST’s response contract remains incorrect — exportapi.md:121. Active exporting returns 208; 409 is the separate AlreadyQueued case. An existing export returns 303 with a URL header, or 204 with noRedirect=true, never the documented 200. Also qualify line 102’s “no action” promise: force=true requests regeneration. See core/common/mixins.py:1085–1100 and core/sources/views.py:618–627.

  9. [P2] DELETE incorrectly prohibits HEAD exports — exportapi.md:139. Authorized users can delete HEAD exports under the same HEAD permission gate; the handler returns 204 when present or 404 when absent. See core/common/mixins.py:1102–1118. The response examples should also say 204 No Content and 404 Not Found.

  10. [P2] Subscriptions still specifies the wrong download protocol and archive format — subscriptions.md:22, repeated at lines 26, 49–51 and 258. GET supplies a Location redirect, not an exportURL/exportUrl header, and the download is ZIP, not compressed tar. Following these instructions would prevent clients from locating or unpacking the export. See core/common/mixins.py:1064 and core/common/utils.py:359–360.

  11. [P3] Signed URLs are described as single-use without implementation support — subscriptions.md:26. The adapters implement expiring URLs, with no consumption or invalidation after a download. Recommend obtaining a fresh URL without implying single-use semantics.

The reference checkout acquired external, uncommitted changes during review. These add two further mismatches if those changes are included in oclapi2#929:

  1. [P2] Version IDs are no longer reproduced exactly — exportapi.md:24. The updated helper replaces characters outside [A-Za-z0-9._@-] with -; an isolated probe converts 1.0 beta to 1.0-beta.

  2. [P2] The filename header is no longer unconditional — exportapi.md:60. The updated helper returns None for keys without a recognized timestamp. Both adapters then omit the disposition override, so neither the advertised filename nor its query parameter is guaranteed.

Overall verdict: Changes requested. The 302 redirect correction and standard filename examples are sound, but the remaining contract errors would mislead API and subscription clients.

…source, HEAD permissions

From Codex pass 1 on ocl-docs#68: 208 vs 204 and 500 on GET; don't follow
the redirect when polling with HEAD; expansion only when a collection has a
default one; lastUpdated comes from the cached export and needs converting
for updatedSince; unsafe version characters become '-'; HEAD exports are for
staff and the repository's owner on GET, POST and DELETE.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@paynejd

paynejd commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Replies to Codex pass 1. Fixed in b1391aa, except where noted:

  1. 208 vs 204, and 500 (fixed): the overview and the GET results now list 302, 208 (being created), 204 (no export), 404 and 500.
  2. Not every collection has an expansion (fixed): the page now says the expansion appears when the collection version has a default expansion.
  3. Timestamp source (fixed): lastUpdated is described as coming from the cached export, as of when the export was created.
  4. updatedSince format (fixed): the page shows converting 2026-09-30_123456 to 2026-09-30T12:34:56Z.
  5. HEAD permissions (fixed): HEAD exports are for staff and the repository's owner (the owning user or members of the owning organization).
  6. HEAD polling and redirects (fixed): the page says not to follow the redirect when checking availability, because the signed URL is valid only for GET.
  7. DELETE and HEAD exports (fixed): DELETE now carries the same HEAD note as GET and POST.
    12, 13. Unsafe characters, and keys without a timestamp (fixed): both changes from oclapi2#929 b7a4ac7a are documented.

Not fixed here (these were already wrong before this PR and are outside the naming change in OpenConceptLab/ocl_issues#2864; proposed as a follow-up ticket):

  • 2: the is_processing advice in the overview.
  • 8: the POST response codes (208/409/303/noRedirect, and force=true).
  • 10, 11: subscriptions.md describes an exportURL header, a tar archive and single-use URLs. The real flow is a Location redirect to a zip at an expiring URL.

…superusers, empty-version timestamp

Addresses Codex pass 2 on ocl-docs#68.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex adversarial review, pass 2 (codex-cli 0.160.1, commit b1391aa)

Reviewed against oclapi2#929 (models.py, mixins.py, aws.py, minio.py are in oclapi2).

Reviewed origin/main...HEAD at b1391aa against API commit b7a4ac7a, excluding the later uncommitted API edits. No files modified.

  1. [P2] Filename headers are not guaranteed for legacy caches — exportapi.md:63
    When the key has no parseable timestamp, get_export_download_name() returns None; both storage backends then omit the disposition override entirely (models.py:1421, aws.py:71, minio.py:96–102). Consequently, neither the header nor the signed URL parameter promised here is guaranteed. Line 27 should describe “no filename override” rather than guarantee a saved storage filename; the client determines the fallback.

  2. [P2] “Other users get 405” overlooks earlier permission checks — exportapi.md:61, also lines 110 and 142
    get_object() checks object permissions before reaching the HEAD gate (mixins.py:1044). An authenticated outsider requesting a private repository receives 403, not 405; unauthenticated requests fail authentication first. Also, the gate explicitly permits is_superuser, independently of staff status or ownership (mixins.py:1032). Qualify 405 as applying to authenticated users with view access who fail the HEAD-specific gate, and include superusers.

  3. [P3] Timestamp description omits empty-repository fallback — exportapi.md:25
    last_child_update falls back to the repository version’s updated_at, then timezone.now(), when both child timestamps are absent (models.py:842–847). This occurs for empty repositories and collections without an expansion. The timestamp therefore does not always represent a concept or mapping change; document that fallback.

The redirect, GET availability statuses, expansion condition, sanitization, and ISO timestamp conversion fixes otherwise match the reference code.

Overall verdict: Needs changes — two P2 findings and one P3 clarification remain.

@paynejd

paynejd commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Replies to Codex pass 2. All three are fixed in d96fa68:

  1. Exports without a name (fixed): the Content-Disposition note now begins "When the API names the export". The legacy bullet says the API sends no filename in that case and the client chooses one.
  2. Who gets 405 (fixed): the HEAD note now includes superusers and limits the 405 to users who can see the repository, on GET, POST and DELETE. Users who can't see it get the usual authentication or permission errors.
  3. Empty versions (fixed): lastUpdated falls back to the version's last update when it has no concepts or mappings, and the page now says so.

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex adversarial review, pass 3 (codex-cli 0.160.1, commit d96fa68)

No remaining or new [P0]–[P3] findings.

Verified the pass 1/2 fixes against the specified oclapi2 implementation and checked subscriptions consistency, respecting your exclusions. No files modified.

Overall verdict: Approve.

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.

1 participant