3.15 not backwards compatible with 3.14 - "View' should either include a `queryset` attribute, or override the `get_queryset()` method." about django-rest-framework

[mrzorn]

I guess my concern is largely with the fact that it didn't follow the deprecation policy, but also with the change itself.

Our code base is pretty extensively tested, but we generally do not go out of our way to test the core functionality in the libraries we use. The fact that there was this breaking change when there was, according to the policy, not supposed to be any, now has me concerned about other changes that might break something in our API that are not as obvious.

Perhaps having more, smaller, releases could help mitigate some of these issues too. As you say, bundling over a year's worth of updates at once makes for a lot of potential places for issues to arise.

When it comes to the specific change, there are many cases where it doesn't necessarily even make sense to specify the queryset, the most obvious one being when the result of get_object is the user on the request, but of course there are many others. There may be cases where get_object is not even returning a django model object, but maybe a dict or some other type. Forcing the queryset to be defined in some of these cases just seems unintuitive.

I am obviously not as familiar with the overall code in the library, so I won't argue what should or should not be done, but it seems like some of the use cases were potentially overlooked, and while defining the queryset might be the optimal solution, making it required in these cases seems perhaps unncessary.

[tomchristie]

While I agree that it didn't really follow the advertised deprecation policy, in the case of the 3.15 release, it was a long time coming, and having 1.5 years separating it with the previous release was bound to contain unintended breaking changes...

That's a really concerning statement. Mature software with few releases generally is extraordinarily stable, not extraordinarily unstable.

So... I'm in complete agreement here.

We've got two different aspects to be addressed here...

• Releasing a 3.15.1 version, with appropriate fixes in place.
• Ensuring that we make the project PR policy really clear, and ensure that REST framework isn't accepting further changes other than security related fixes and changes required by newer Django versions.

[peterthomassen]

So if I read that correctly, the error message is not really an issue anymore,

For those who happen to have "fixed" their downstream code. It is still a DRF regression.

but your concern is more around the fact that it was a breaking change despite the major digit of DRF not having changed? Would adding this to the release note be enough to close this issue?

Not at all! A patch release with a fix in DRF is required, or alternatively the release should be withdrawn and re-released with a major version bump.

While I agree that it didn't really follow the advertised deprecation policy, in the case of the 3.15 release, it was a long time coming, and having 1.5 years separating it with the previous release was bound to contain unintended breaking changes...

That's a really concerning statement. Mature software with few releases generally is extraordinarily stable, not extraordinarily unstable.

There's another regression caught by our application-side tests. I haven't had time to report it, but am planning to do so later this week.

In my view, this is a symptom of a larger issue, as it suggests that DRF has insufficient test coverage.

The fix I suggest would work in simple case, but would still require you to override the queryset in case of prefetches, and it's probably OK. In more complex cases, the error message tells exactly what is wrong, and user can easily fix it.

The code should simply gracefully handle (the exception in) scenarios where the queryset isn't defined or not of the expected type.

[mrzorn]

While I agree that it didn't really follow the advertised deprecation policy, in the case of the 3.15 release, it was a long time coming, and having 1.5 years separating it with the previous release was bound to contain unintended breaking changes...

That's a really concerning statement. Mature software with few releases generally is extraordinarily stable, not extraordinarily unstable.

So... I'm in complete agreement here.

We've got two different aspects to be addressed here...

• Releasing a 3.15.1 version, with appropriate fixes in place.
• Ensuring that we make the project PR policy really clear, and ensure that REST framework isn't accepting further changes other than security related fixes and changes required by newer Django versions.

This is good to hear.

My biggest concern was not that I had to update my code with a new release, its that this was a point release that should not have required me to do so.

I am most concerned about what other updates were made that may cause issues that are not as immediately obvious, and it seems like there are at least a few others looking at the issue list.

The whole situation has left me less confident in future DRF releases and considering rolling back to 3.14 as I know that it is stable and have much higher confidence in it behaving as I expect.

[browniebroke]

In the end, I already updated all of the relevant code to define the queryset on my views, but the nature of the breaking change is concerning for others using the library.

So if I read that correctly, the error message is not really an issue anymore, but your concern is more around the fact that it was a breaking change despite the major digit of DRF not having changed? Would adding this to the release note be enough to close this issue?

While I agree that it didn't really follow the advertised deprecation policy, in the case of the 3.15 release, it was a long time coming, and having 1.5 years separating it with the previous release was bound to contain unintended breaking changes...

The fix I suggest would work in simple case, but would still require you to override the queryset in case of prefetches, and it's probably OK. In more complex cases, the error message tells exactly what is wrong, and user can easily fix it.

[kevinrenskers]

While I agree that it didn't really follow the advertised deprecation policy, in the case of the 3.15 release, it was a long time coming, and having 1.5 years separating it with the previous release was bound to contain unintended breaking changes...

How can there be unintended breaking changes in a point release? This is extremely concerning.

[mrzorn]

So if I read that correctly, the error message is not really an issue anymore,

For those who happen to have "fixed" their downstream code. It is still a DRF regression.

but your concern is more around the fact that it was a breaking change despite the major digit of DRF not having changed? Would adding this to the release note be enough to close this issue?

Not at all! A patch release with a fix in DRF is required, or alternatively the release should be withdrawn and re-released with a major version bump.

Definitely agreed here - this is a regression and should be addressed.

[nrlulz]

We just ran into this too. Also interested to know if it was intentional - I feel like this has got to be a pretty common pattern?

Related PR: #8043

I worked around it for now by just adding ModelClass.objects.none() to affected view classes, e.g.:

 class MeView(generics.RetrieveUpdateAPIView):
+    queryset = User.objects.none()
     serializer_class = MeSerializer
 
     def get_object(self):
         return self.request.user

[lorenzocelli]

Same here

[browniebroke]

What is your get_object() method looking like? I started to look at a potential fix (#9314) but after opening the pull request I noticed that I made a pretty significant assumption, that the get_object method isn't doing any prefetches.

[mrzorn]

What is your get_object() method looking like? I started to look at a potential fix (#9314) but after opening the pull request I noticed that I made a pretty significant assumption, that the get_object method isn't doing any prefetches.

It is sometimes as simple as return request.user or potentially doing a get_or_create or a query on a queryset with many filters/select_related/prefetch_related/etc - there isn't one set pattern I have here.

I don't totally understand the rationale behind clearing and refetching the related objects, but perhaps it is something that is just documented as the best practice, but does not throw an error if the queryset property or get_queryset method is not defined?

In the end, I already updated all of the relevant code to define the queryset on my views, but the nature of the breaking change is concerning for others using the library.