Opened 13 months ago

Closed 10 months ago

Last modified 10 months ago

#22959 closed Cleanup/optimization (fixed)

Document that custom validator must be comparable to prevent infinite migrations

Reported by: lorinkoz@… Owned by: Markush2010
Component: Documentation Version: 1.7-rc-1
Severity: Normal Keywords: migrations
Cc: areski, info+coding@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I'm using a custom validator with a proper deconstruct on a TextField, it's intended to control the min and max number of words in the field. If I run python manage.py makemigrations over and over again it keeps generating the same migration related to changes on that text field, changes that do not exist.

Change History (11)

comment:1 Changed 13 months ago by timo

  • Component changed from Migrations to Documentation
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from Custom validator in TextField to Document that custom validator must be comparable to prevent infinite migrations
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

This probably isn't an issue with Django itself but rather with your validator; they need to be comparable as in a68f32579145dfbd51d87e14f9b5e04c770f9081. However, we could add some docs about this since it will probably be a FAQ.

comment:2 Changed 12 months ago by areski

I have been trying to reproduce this problem without luck!
Could you share a code sample where this could be reproduced?

This is how I tried to reproduce it:

from django.db import models
from django.core.exceptions import ValidationError


def validate_textlimit20chars(value):
    if len(value) > 20:
        raise ValidationError('%s is not an even number' % value)


class MyModel(models.Model):
    name = models.CharField(max_length=32)
    text20_field = models.TextField(default="", validators=[validate_textlimit20chars])

comment:3 Changed 12 months ago by areski

  • Cc areski added

comment:4 Changed 12 months ago by timgraham

Try a class-based validator like the ones for which an __eq__ method was added in the commit I linked in comment 1.

comment:5 Changed 12 months ago by areski

I'm trying to figure out how to make the class-based validator works, no luck: https://gist.github.com/areski/b9e293ad34d2b6845958
anyone can provide a working example?

comment:6 Changed 12 months ago by lorinkoz

Here's the original validator I used to get to know the problem (of course, by the time I discovered it there was no cmp)

class LimitWordsValidator:
    
    def __init__(self, limit, upper=True):
        self.limit = limit
        self.upper = upper
    
    def __cmp__(x, y):
        return cmp(x.limit, y.limit) or cmp(x.upper, y.upper)
    
    def __call__(self, value):
        words = len(strip_tags(value).split())
        args = {
            'limit': self.limit,
            'current': words,
        }
        if self.upper and words > self.limit:
            raise ValidationError(_('You cannot exceed %(limit)s words, you have %(current)s.') % args)
        if not self.upper and words < self.limit:
            raise ValidationError(_('You need at least %(limit)s words, you have %(current)s.') % args)
    
    def deconstruct(self):
        return (
            'common.validators.LimitWordsValidator',
            [self.limit],
            {'upper': self.upper},
        )

comment:7 Changed 10 months ago by Markush2010

This is similar to #23473

comment:8 Changed 10 months ago by Markush2010

  • Cc info+coding@… added
  • Owner changed from nobody to Markush2010
  • Status changed from new to assigned

comment:9 Changed 10 months ago by Markush2010

  • Has patch set

I added a pull request with documentation updates: https://github.com/django/django/pull/3271

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

  • Resolution set to fixed
  • Status changed from assigned to closed

In c692e37b6350171ee2e04b3e7090babf34ac140b:

Fixed #22959 -- Documented that class-based validators need to be deconstructible.

comment:11 Changed 10 months ago by Tim Graham <timograham@…>

In 5356183716daa88e5c0009b179de55370bd5c5da:

[1.7.x] Fixed #22959 -- Documented that class-based validators need to be deconstructible.

Backport of c692e37b63 from master

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