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

Investigate how responses are handled in testing #72

Open
jjnesbitt opened this issue Aug 25, 2021 · 5 comments
Open

Investigate how responses are handled in testing #72

jjnesbitt opened this issue Aug 25, 2021 · 5 comments

Comments

@jjnesbitt
Copy link
Member

It appears that in a testing context (when using api_client or authenticated_api_client), data is directly returned from an api endpoint, without serialization. For example, if you directly return an arangodb cursor from an endpoint, inspecting r.data shows that the type is Cursor. This is in contrast to what we expect from the endpoint, which is serialized JSON. In my example, we can still call r.json() and it works as expected, but IMO this is still a slight issue, as it represents a difference in our test cases vs reality.

This doesn't usually crop up due to our use serializers in most places, but for the rare cases where we don't use serializers, this could cause issues. If nothing else we should be aware of this behavior, but we should also see what we can do to mitigate this.

@jjnesbitt
Copy link
Member Author

@waxlamp
Copy link
Collaborator

waxlamp commented Aug 31, 2021

When don't we use serializers?

@naglepuff
Copy link
Collaborator

At least one endpoint:
GET /api/workspaces/{workspace}/aql/
doesn't use a serializer for the response. It just creates a response object and passes in the Cursor object returned as a result of executing the query with python-arango. @AlmightyYakob might know of another place where we don't use serializers. For this particular case, it might be possible to use a ListSerializer with a child JSONSerializer. However, in this case we don't necessarily know what the result will look like. If an AQL query evaluates to a list of string values, I'm not sure how JSONSerializer would handle that.

@waxlamp
Copy link
Collaborator

waxlamp commented Aug 31, 2021

I don't think we should use serializers simply because we're facing the problem outlined in this issue (without further understanding of the problem), but just to be nerdy: a single string is in fact a basic JSON value, and therefore a ListSerializer with a JSONSerializer should (on the face of it) be just fine.

@AlmightyYakob, are there other places where we lack a serializer? And in the case of the AQL endpoint, what is the magic that allows for constructing a Response with a python-arango Cursor?

@jjnesbitt
Copy link
Member Author

Another place we don't use serializers is in the table get_rows endpoint, as well as the endpoints for retrieving network nodes and edges. The response is paginated however, so it's not returning a cursor directly. I'm not sure what allows for the cursor to be returned directly, but I'm guessing somewhere in django's request lifecycle, it converts the iterable to a list/json.

Initially, I was thinking it was best to just return the cursor directly, since it works in Django, and it's the most direct path. The main reason for me opening this issue is that since it works in the general django response, I'd like the same response to exist in testing. We could use a serializer in this response, and maybe we should, but if it's just an issue with testing, I'd like to try to solve it there, rather than change our endpoint behavior to accommodate testing. From the docs I linked, it looks like there may be a valid approach there. However, I wouldn't be opposed to using a serializer in that endpoint directly either, so long as it doesn't come with serious disadvantages (which I doubt it will).

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

No branches or pull requests

3 participants