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

Don't error on non existing version #6325

Merged
merged 1 commit into from Nov 6, 2019

Conversation

@stsewd
Copy link
Member

stsewd commented Oct 23, 2019

if not kwargs['filters']['version']:
raise ValidationError("Unable to find a version to search")
log.info("Unable to find a version to search")
return HTMLFile.objects.none()

This comment has been minimized.

Copy link
@humitos

humitos Oct 28, 2019

Member

Some lines below, we are returning a PageSearch object. Shouldn't we return something similar here instead of a different queryset object?

This comment has been minimized.

Copy link
@stsewd

stsewd Oct 28, 2019

Author Member

PageSearch isn't an django model, is an elastic search document based on HTMLFile. Actually it's weird becuase the PagaSearch has an interface similar to what django rest framework expects (there is a comment for that in this class or somewhere else). Also, funny that returning an empty PageSearch doesn't work with django rest framework.

From what I saw, we can return real HTMLFile objects, but that requires a query to the db, so I guess that's why we are not doing that and doing this hacky thing.

This comment has been minimized.

Copy link
@dojutsu-user

dojutsu-user Oct 30, 2019

Member

There's also an open issue related to this: #5235

@stsewd stsewd force-pushed the stsewd:dont-error-on-wrong-input branch from 68de6a6 to 4817473 Oct 28, 2019
@humitos humitos requested a review from dojutsu-user Oct 30, 2019
Copy link
Member

dojutsu-user left a comment

Looks good to me.

@ericholscher ericholscher merged commit f30eb28 into readthedocs:master Nov 6, 2019
2 checks passed
2 checks passed
continuous-documentation/read-the-docs Read the Docs build succeeded!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@stsewd stsewd deleted the stsewd:dont-error-on-wrong-input branch Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.