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

3.15 Release #9210

Merged
merged 71 commits into from
Mar 16, 2024
Merged

3.15 Release #9210

merged 71 commits into from
Mar 16, 2024

Conversation

tomchristie
Copy link
Member

Reviewing #8794.

Starting point, I've reviewed the release notes, and cleaned up items that shouldn't be included...

  • Docs changes should not be part of our release notes.
  • Project workflow changes should not be part of our release notes.
  • Test changes should not be part of our release notes.

Our release notes should reflect changes to the released package version.
So... behavioural changes that users may need to be aware of.

Project documentation, workflow, and testing isn't part of that and shouldn't be part of our release notes.

math-a3k and others added 30 commits September 13, 2023 11:34
- Add docs/community/3.15-announcement.md
- Update docs/community/release-notes.md
- Update mkdocs.yml

Co-authored-by: Bruno Alla <[email protected]>
@tomchristie
Copy link
Member Author

Anything else blocking us from merging this?

@TGoddessana
Copy link
Contributor

Should this milestone be resolved and merged?

@browniebroke
Copy link
Contributor

Anything else blocking us from merging this?

Not sure if a blocker, but you might want to update that sentence with -at least- an updated date:

At the Internet, on January 12th, 2024, with 135 commits 107 authors, we are happy to announce the release of Django REST framework 3.15.

@tomchristie
Copy link
Member Author

you might want to update that

Okay, someone want to propose a release date and make a +/- suggestion?

@auvipy auvipy self-requested a review March 10, 2024 11:58
@TGoddessana
Copy link
Contributor

This is the final step to release, right? If so, the sooner the better, but I think you'll want to review your release process and set your own time to release.

@tomchristie
Copy link
Member Author

Okay, I'd suggest March 15th... perhaps someone could issue a +/- suggestion and we plan on making the release then?

docs/community/3.15-announcement.md Outdated Show resolved Hide resolved
* Fixes `BrowsableAPIRenderer` for usage with `ListSerializer`. [[#7530](https://github.com/encode/django-rest-framework/pull/7530)]
* Change semantic of `OR` of two permission classes [[#7522](https://github.com/encode/django-rest-framework/pull/7522)]
* Remove dependency on `pytz` [[#8984](https://github.com/encode/django-rest-framework/pull/8984)]
* Fix validation for ListSerializer [[#8979](https://github.com/encode/django-rest-framework/pull/8979)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#8979 is marked as "breaking change" and the fix for that (#9244) is not merged yet. It should be noted in the release notes this is a breaking-change.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#8979 is marked as "breaking change" and the fix for that (#9244) is not merged yet. It should be noted in the release notes this is a breaking-change.

@TGoddessana please see same implemented @, thank you,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I'm not a maintainer, I'm very interested in the maintenance of the project, so I took a look at the issues.

  1. one PR (Fix validation for ListSerializer #8979) that solves the original issue (Invalid self.instance when validating the serializer using many=True #8926) seems to have been merged already, but the better implementation (Improve ListSerializer #9244) is still not fully resolved based on the last comment in the issue.

  2. so which decision do we make?

Would love to hear your thoughts @tomchristie 🥲

Copy link
Contributor

@sevdog sevdog Mar 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the problem is that #8926 was addressed ignoring the advice regarding "Customizing multiple update"

By default the ListSerializer class does not support multiple updates. This is because the behavior that should be expected for insertions and deletions is ambiguous.

To support multiple updates you'll need to do so explicitly. When writing your multiple update code make sure to keep the following in mind:

  • How do you determine which instance should be updated for each item in the list of data?
  • How should insertions be handled? Are they invalid, or do they create new objects?
  • How should removals be handled? Do they imply object deletion, or removing a relationship? Should they be silently ignored, or are they invalid?
  • How should ordering be handled? Does changing the position of two items imply any state change or is it ignored?

You will need to add an explicit id field to the instance serializer. The default implicitly-generated id field is marked as read_only. This causes it to be removed on updates. Once you declare it explicitly, it will be available in the list serializer's update method.

Thus the proposed solution (#8979) only addesses a single use case while ignoring out many others which are now not supported by the framework. For example: using ListSerializer to perform in a bulk request a "create-update-delete" of a list of objects which was possible before this change.

PS: there is also an issue with the current implementation of ListSerializer when validating instances and just use "index" to choose which instance will be associated to which data.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about reverting that for this release

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#9283 let me know your thoughts, and we can wait for a better fix in future

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this change is not released in 3.15, which release do you think is the right one to fix and release it in: 3.16 or 3.15.1?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it is just reverted it will not be included in 3.15, as it is a big thing, it should be included in a major release like 3.16/3.17

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to avoid including any not fully functional feature for 3.15

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to avoid including any not fully functional feature for 3.15

I agree with you. Thank you for your efforts!

docs/community/release-notes.md Outdated Show resolved Hide resolved
@tomchristie
Copy link
Member Author

Okay, I think we're good to release here?

@tomchristie tomchristie changed the title Release notes for 3.15 3.15 Release Mar 15, 2024
@tomchristie
Copy link
Member Author

tomchristie commented Mar 15, 2024

Okay, I might have enough time to roll this release later today... (on schedule ☺️) unless there's any objections?
Minor query from my perspective... are we okay with the supported range of Python & Django, or should we keep them more constrained?... (Example... We could bump up the formal support to Django 4.0 and Python 3.8 in our docs without actually changing anything else)

@cclauss
Copy link
Contributor

cclauss commented Mar 15, 2024

https://docs.djangoproject.com/en/stable/faq/install/#what-python-version-can-i-use-with-django still supports Django 3.2 (at least until next month).

However, I think we could safely drop support for Python < 3.8 (https://devguide.python.org/versions) because supporting five different versions of CPython is enough effort.

Perhaps release this as is and then in a few months create a release that drops Django v3.x and also Python < 3.8.

Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should just keep supporting as many versions as possible in this release as it is a long overdue. after that we can decide which versions we should drop.

@auvipy auvipy added this to the 3.15 milestone Mar 15, 2024
@tomchristie tomchristie merged commit 2d8e9ad into master Mar 16, 2024
10 checks passed
@tomchristie tomchristie deleted the math-a3k-315_rn branch March 16, 2024 17:21
@tomchristie
Copy link
Member Author

Okay, we're merged add ready to publish.
The deploy to pypi will need to wait until monday now...

@tfranzel
Copy link
Member

Thank you guys for pushing this out the door. Much appreciated!

Now that the pypi release is done, we should probably also craft a github release for 3.15.0.

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

Successfully merging this pull request may close these issues.