fix(datashare-python): align pagination with spec - #68
Conversation
73c01ad to
3974c4b
Compare
0b27664 to
43b1e0f
Compare
pirhoo
left a comment
There was a problem hiding this comment.
Nice tightening of the pagination shape, and moving total up into a wrapper does read closer to the spec. Two blockers below sit in workers/extract-worker/, which this PR does not touch, so GitHub will not let me anchor them inline. They are the reason I am requesting changes.
workers/extract-worker/extract_worker/activities.py · lines 221-224
crucial: total moved off BasePagination, and icij_config() sets extra="forbid", so this call now raises ValidationError: total / Extra inputs are not permitted. That kills every successful doc in extract_markdown_content_act, and CI runs the extract-worker suite on any datashare-python/** change. Could we wrap it in the new Pages instead (and add Pages to the datashare_python.objects import at the top)?
pages = extract_res.output.pages
pages = Pages(
total=pages.total,
pagination=ByteRangesPagination(byte_ranges=pages.byte_ranges),
)workers/extract-worker/extract_worker/objects.py · line 50
important: paired with the above, the manifest field needs the wrapper type too, otherwise total is silently dropped from manifest.json and entry.pages.total in test_extract_markdown_content_act stops resolving. Swap ByteRangesPagination for Pages in the import as well.
pages: Pagesbb8d0e1 to
7378217
Compare
7378217 to
6ea1abf
Compare
solves #67