Skip to content

Reject page_size < 1 and csv/tsv records_format in get_datastore_records - #118

Merged
mcarans merged 1 commit into
mainfrom
datastore-records-arg-validation
Oct 15, 2026
Merged

mcarans merged 1 commit into
mainfrom
datastore-records-arg-validation

Conversation

@mcarans

@mcarans mcarans commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

get_datastore_records now raises HDXError for two inputs it cannot handle:

  • page_size below 1. With page_size=0, CKAN returns no records and the loop check 0 < 0 is false, so it requests offset 0 forever. On stage, it was stopped after 10 identical calls.
  • records_format csv or tsv. CKAN returns records as a single string for these formats, so yield from records yields single characters and len(records) counts characters, not rows. On stage, a 27-row resource with page_size=10 yielded 7,795 characters. search_datastore handles these formats correctly and remains the way to get them.

Both errors are raised on the first iteration, since the method is a generator. Neither case worked before, so no working caller is affected. The docstring and documentation/index.md note the csv/tsv restriction.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Test Results

180 tests  ±0   180 ✅ ±0   1m 15s ⏱️ -20s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 32b5932. ± Comparison against base commit 6832186.

♻️ This comment has been updated with latest results.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 97.259% (+0.004%) from 97.255% — datastore-records-arg-validation into main

@mcarans
mcarans merged commit 3a6926c into main Oct 15, 2026
6 of 7 checks passed
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.

2 participants