[PATCH 3/3] Remove 'PatchRelationSerializer'
Stephen Finucane
stephen at that.guru
Mon Nov 30 00:15:42 AEDT 2020
On Fri, 2020-10-09 at 10:51 +1100, Daniel Axtens wrote:
> Stephen Finucane <stephen at that.guru> writes:
>
> > This wasn't writeable for reasons I haven't been able to figure out.
> > However, it's not actually needed: the 'PatchSerializer' can do the job
> > just fine, given enough information. This exposes a bug in DRF, which
> > has been reported upstream [1]. While we wait for that fix, or some
> > variant of it, to be merged, we must monkey patch the library.
> >
> > [1] https://github.com/encode/django-rest-framework/issues/7550
> > [2] https://github.com/encode/django-rest-framework/pull/7574
>
> ISTR there being performance reasons I wrote this, or I just couldn't
> bend DRF to my will. I'll double-check.
>
> But more to the point I am _extremely_ uncomfortable monkey-patching a
> dependency. At the absoulute very least we need to check the version of
> DRF before we monkey-patch it, but ideally we shouldn't be
> monkey-patching at all.
>
> I know, I know, I once proposed doing the same thing to get around a
> performance bug in Django. I was 3 years younger and dumber, but to
> quote your concerns at the time:
>
> > OK, so I know this is a pretty serious performance issue but I really don't
> > want to merge this :( It's far too...hacky, as you say above, and pretty
> > unmaintainable to book.
I've worked through a few other options today but unfortunately this is as good
as I can do. The fix is sound, being based on what DRF itself does for other
fields (see the PR) and I do have it proposed to DRF so we won't have to carry
it forever. I could add a version check that will simply error out for newer
versions of DRF, but tbh we already do that through our requirements files and
anyone running with other package versions is already using an unsupported
configuration. I think this should go as-is. Hopefully we can revert it in the
near future once my PR gets some attention (post-COVID, probably).
Cheers,
Stephen
> Kind regards,
> Daniel
More information about the Patchwork
mailing list