Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValueError for non-int limit and offset query params #1258

Merged
merged 9 commits into from
Dec 10, 2013

Conversation

joetsoi
Copy link
Contributor

@joetsoi joetsoi commented Nov 5, 2013

These lines obviously cause problems for non-int query params. This hit me as a 500 in production.

Since this is at least the third time for this kind of error to pop up, I'm strongly suggesting a proper pagination implementation to be used across resources. Something along the lines of:

pagination = get_pagination(data_dict, max_limit=config.get('ckan.activity_list_limit', 31))
activity_objects = model.activity.user_activity_list(user.id, **pagination.get_kwargs())

There are many other places in get.py where this is a problem.

@ghost ghost assigned joetsoi Oct 3, 2013
 * Add decorator that validates against a given schema
 * Add default_activity_list_schema
 * Refactor user_activity list to use validator decorator
change tests, no need to hit the database we only need to test that the
vaildators raise a ValidationError
@joetsoi
Copy link
Contributor

joetsoi commented Nov 5, 2013

@joetsoi
Copy link
Contributor

joetsoi commented Nov 20, 2013

'convert_to_json_if_string' seems a bit ridiculous but I wanted to keep the logic the same and something like 'is_json' would be misleading. :(

@ghost ghost assigned davidread Nov 21, 2013
@ghost ghost assigned kindly Dec 10, 2013
@davidread
Copy link
Contributor

Review - all looks good.

Well worth having all these improvements in. A few get functions remain without the validation still - group_package_show, tag_search, tag_autocomplete, but these can be done in the future.

I'm handing off to @kindly and recommending it is merged.

kindly added a commit that referenced this pull request Dec 10, 2013
ValueError for non-int limit and offset query params
@kindly kindly merged commit 814745a into master Dec 10, 2013
@smotornyuk smotornyuk deleted the 1258-validator-decorators branch December 19, 2018 14:52
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.

None yet

3 participants