Add CursorPagination, enhance test client to handle client.post better #322
Conversation
|
👍 |
erichaus
changed the title
| @@ -396,7 +396,7 @@ def build_json_resource_obj(cls, fields, resource, resource_instance, resource_n | |||
| resource_data.append(('links', {'self': resource[api_settings.URL_FIELD_NAME]})) | |||
| return OrderedDict(resource_data) | |||
|
|
|||
| def render_relationship_view(self, data, accepted_media_type=None, renderer_context=None): | |||
| def render_relationship_view(self, data, accepted_media_type=None, renderer_context={}): | |||
There was a problem hiding this comment.
Using a mutable default parameter is generally a bad idea in python, due to:
Python's default arguments are evaluated once when the function is defined, not each time the function is called (like it is in say, Ruby). This means that if you use a mutable default argument and mutate it, you will and have mutated that object for all future calls to the function as well.
There was a problem hiding this comment.
@santiavenda2 fair point, I guess this package needs to add better exception handling then since right off the bat if renderer_context is the default of None, the first line of code will fail. We're looking at other options though as this package lacks documentation and doesn't appear to be active anymore. The added complexity layer on top of DRF which is on top of Django just makes it very hard to deal with.
There was a problem hiding this comment.
Does this mean you're ok closing this PR? This comment makes it seem to me like this is not important to you anymore, but I might be misreading. I'm wondering if I should spend the time to review or not.
Opening this preemptively for discussion:
Aside from adding a cursor paginator I think we need to address an issue that was already brought up by @abdulhaq-e and others where testing code that uses this package will break on
APITestClient().postas discussed here #275Will resolve #321 and #320