Unique constraint fails when trying to bulk update #30
Comments
|
What's your traceback?
|
|
Environment: Request Method: PUT Django Version: 1.7.4 Traceback:
Exception Type: AttributeError at /api/1.0/q/answers |
|
Don't see anything wrong. Will need to debug. |
|
I debugged it and saw that the problem was that instance is a QuerySet. Maybe is a DRF problem? Thanks for fast reply. |
|
The issue sounds like DRF is assuming you are working with a single object, but not a queryset. The issue though is that we are making it clear that it's multiple objects. I'd have to look further into the traceback to determine the actual cause though. |
|
Seems like this is an issue in DRF itself. It blows up here. |
|
Just opened PR in DRF with a preliminary fix - encode/django-rest-framework#2575 Please let me know if that fixes your problem. |
|
Indeed it fixes my issue. Thank you very much. |
|
Looking at PR #2575 in DRF itself, and then at PR #2720, I can see where the problem is, but I still don't know what the fix is, since @tomchristie said that he will not accept that pull request because that is not general enough. |
|
Agreed this is a bug with DRF. While waiting for a DRF fix (I scanned through the DRF issue list and didn't see a matching entry for this bug), what if you did the following: Instead of validating the list serializer in it's entirety, iterate over each item in the list and validate it individually. As each item is validated, the validated data can be captured in a running validated data list. Once every item has been validated, the list of validated data is set as the validated data on the original list serializer. The concept is outlined below. NOTE: To do this properly, you would not raise an exception immediately if an individual item fails validation as I have done, but rather capture the errors from each item that fails and return a complete error response for the whole list. def bulk_update(self, request, *args, **kwargs):
partial = kwargs.pop('partial', False)
# restrict the update to the filtered queryset
serializer = self.get_serializer(
self.filter_queryset(self.get_queryset()),
data=request.data,
many=True,
partial=partial,
)
validated_data = []
for item in request.data:
item_serializer = self.get_serializer(
data=item,
partial=partial,
)
item_serializer.is_valid(raise_exception=True)
validated_data.append(item_serializer.validated_data)
serializer._validated_data = validated_data
self.perform_bulk_update(serializer)
return Response(serializer.data, status=status.HTTP_200_OK) |
|
Jumpin' in. @mstachowiak this is a nice fix, I'm using it, actually and added error catching. There was another improvement to make for it to work, though: pass the updated instance to the item_serializer (otherwise it acts as if you want to create the object and not update it). def bulk_update(self, request, *args, **kwargs):
partial = kwargs.pop('partial', False)
# restrict the update to the filtered queryset
serializer = self.get_serializer(
self.filter_queryset(self.get_queryset()),
data=request.data,
many=True,
partial=partial,
)
validated_data = []
validation_errors = []
for item in request.data:
item_serializer = self.get_serializer(
self.get_queryset().get(pk=item['id']),
data=item,
partial=partial,
)
item_serializer.is_valid(raise_exception=True)
if item_serializer.errors:
validation_errors.append(item_serializer.errors)
validated_data.append(item_serializer.validated_data)
if validation_errors:
raise ValidationError(validation_errors)
serializer._validated_data = validated_data
self.perform_bulk_update(serializer)
return Response(serializer.data, status=status.HTTP_200_OK)Since it is on hold (and looks a little like a dead end...) on django-rest-framework side maybe the patch can be added directly to drf-bulk, what do you think @miki725 ? |
|
@adelaby thanks for extended code example. I would however like to suggest a couple of corrections:
Corrected code: def bulk_update(self, request, *args, **kwargs):
partial = kwargs.pop('partial', False)
# restrict the update to the filtered queryset
serializer = self.get_serializer(
self.filter_queryset(self.get_queryset()),
data=request.data,
many=True,
partial=partial,
)
validated_data = []
validation_errors = []
for item in request.data:
item_serializer = self.get_serializer(
get_object_or_404(self.filter_queryset(self.get_queryset()), pk=item['id']),
data=item,
partial=partial,
)
if not item_serializer.is_valid():
validation_errors.append(item_serializer.errors)
validated_data.append(item_serializer.validated_data)
if validation_errors:
raise ValidationError(validation_errors)
serializer._validated_data = validated_data
self.perform_bulk_update(serializer)
return Response(serializer.data, status=status.HTTP_200_OK) |
|
Considering that this bug is caused by the for item in data:
try:
if self.instance is not None:
id_attr = getattr(self.child.Meta, 'update_lookup_field', 'id')
# there should also be some preemptive checks
# that the data actually has the id
filters = {id_attr: item.get(id_attr, None)}
self.child.instance = self.instance.filter(**filters).first()
validated = self.child.run_validation(item)
except ValidationError as exc:
errors.append(exc.detail)
else:
ret.append(validated)
errors.append({})Correct me if I am wrong, I don't know what the stance is on rewriting DRF methods. I understand their team's decision not to alter it, as the default behaviour of Another less invasive, but more complex option might be to add a custom Any opinions? |
|
|
@kevin-brown Sorry, I think I didn't explain it correctly. I actually meant |
|
'id' key error raises my case since individual serializer run .is_valid() instead of ListSerializer so that I munually add root with BulkListSerializer in order to let BulkSerializerMixin.to_internal_value() add 'id' for 'PUT' and 'PATCH' mode. for item in request.data:
item_serializer = self.get_serializer(
get_object_or_404(self.filter_queryset(self.get_queryset()), pk=item['id']),
data=item,
partial=partial,
)
# add here
item_serializer.root = serializer
if not item_serializer.is_valid(): |
|
Ran into this issue with the sample code provided in the README, while trying to bulk update a basic model (with only a Out of the box, the following seems to be broken (from the README): I'm running Django 1.10.2, Django Rest Framework 3.5.1 and Django Rest Framework-Bulk 0.2.1 |
|
I was also facing |
|
Closing as stale. Can consider reopening if it turns out this is still a current issue. |
I'm trying to use the new bulk update that uses DRF 3. When I try to bulk update with a model that uses a
unique_togetherconstraint I get an error: 'QuerySet' object has no attribute 'pk'Here is the code
I tried to debug the problem and I think it is a validation problem. Plase help.
TY
The text was updated successfully, but these errors were encountered: