Opened 2 years ago

Closed 19 months ago

Last modified 3 months ago

#20439 closed Cleanup/optimization (fixed)

Deprecate IPAddressField in favour of GenericIPAddressField

Reported by: erikr Owned by: Aymeric Augustin <aymeric.augustin@…>
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: dceu13
Cc: eromijn@…, mike@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Since Django 1.4, we've added GenericIPAddressField, next to IPAddressField. The new GenericIPAddressField supports IPv4 as well as IPv6 addresses, and does normalisation of IPv6 addresses. It can also be configured to only accept IPv4 or IPv6 addresses.

As far as I know, IPAddressField has no current features that are not also available in a GenericIPAddressField. Therefore, I suggest that we, some time from now, deprecate IPAddressField, in favour of GenericIPAddressField.

For users, it is database-dependent whether IPAddressFields can just be replaced with GenericIPAddressFields: on PostgreSQL and SQLite, no changes are needed; schema changes are needed on MySQL and Oracle. Examples are listed in the 1.6 release notes https://docs.djangoproject.com/en/dev/releases/1.6/#storage-of-ip-addresses-in-the-comments-app, as we just made the same change for comments.

Change History (12)

comment:1 Changed 2 years ago by erikr

  • Cc eromijn@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 2 years ago by erikr

comment:3 Changed 2 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Someday/Maybe

I'm almost sure we'll do this at some point. The ticket can be moved to Accepted if the discussion concludes that we should do it now.

comment:4 Changed 2 years ago by joejasinski

Hey all, would it be possible to simply keep the old IPAddressField around as a proxy to GenericIPAddressField? I know it adds a bit of redundancy to the code, but would be one fewer backward-incompatible change to manage during upgrades. Additionally, people could continue to use the more concisely named IPAddressField. Just my thoughts...

comment:5 Changed 2 years ago by erikr

I don't think so - they use different database column types on MySQL and Oracle. Worse, taking an IPAddressField column and using it to store GenericIPAddressField data may result in silent truncation.

comment:6 Changed 23 months ago by carbonXT

  • Cc mike@… added

comment:7 Changed 20 months ago by timo

  • Component changed from Forms to Database layer (models, ORM)
  • Triage Stage changed from Someday/Maybe to Accepted

The mailing list thread has several +1's and no objections from core devs.

comment:8 Changed 20 months ago by erikr

  • Owner changed from nobody to erikr
  • Status changed from new to assigned

comment:9 Changed 19 months ago by erikr

  • Has patch set
  • Owner erikr deleted
  • Status changed from assigned to new

PR: https://github.com/django/django/pull/1688

I have added the deprecation to the release notes, the documentation of both IPAddressFields, and in the deprecation timeline. Warnings have been silenced in all relevant tests.

comment:10 Changed 19 months ago by aaugustin

  • Triage Stage changed from Accepted to Ready for checkin

The PR looks good. I only have trivial comments. I'll merge it.

comment:11 Changed 19 months ago by Aymeric Augustin <aymeric.augustin@…>

  • Owner set to Aymeric Augustin <aymeric.augustin@…>
  • Resolution set to fixed
  • Status changed from new to closed

In 1a63092981c685214ea189a14007da7bcc823c17:

Fixed #20439 -- Started deprecation of IPAddressField

comment:12 Changed 3 months ago by Tim Graham <timograham@…>

In 33457cd3b0da69320d3f66bb6d5a673950c5032f:

Removed IPAddressField per deprecation timeline; refs #20439.

Note: See TracTickets for help on using tickets.
Back to Top