Hi Markus,
Sorry for the slow reply, I never received a notification that you'd
posted. (Gmail does weird inbox sorting these days).
Assuming I have an existing model, adding
>
> models.CharField(blank=True, max_length=150)
>
> to it, doesn't invoke the questioner on current master. Changing this
> field to
>
> models.CharField(max_length=150)
>
> doesn't call the questioner either.
>
Ok so that is acceptable (I put question marks because I hadn't checked
behaviour in that case). But the other two cases I listed:
- AddField without `blank=True` or `null=True`.
- AlterField [removing `null=True`] AND [removing `blank=True` OR already
not having `blank=True`]
calls the questioner.
In its current state I don't like this because:
(1) Whether or not the questioner (a backend helper) opens depends on
`blank` (a frontend validation option).
(2) The questioner phrases itself as you trying to do something erroneous
('you can't do this'), when it's a valid thing to want to do.
The behaviour in (1) threw me when i first stumbled across it, as I hadn't
expected `blank` to have any effect on making migrations. Tim has kind of
convinced me to accept it, just because it's the 'usual' scenario and makes
things convenient. Plus, fixing this would either mean reverting #23405
<https://code.djangoproject.com/ticket/23405> (bad option), or dropping the
questioner altogether (correctness over convenience; not a great option
either).
Alternatively, could try to address (2) by making the questioner less
scary: 'Are you sure?/maybe you'd like to set a one-time default instead?'
over 'you can't do this; set a default or quit'.
So I think if anything comes of this, it'd likely just be a slight
rewording of the questioner and an additional choice being added. This
wouldn't offer any new functionality, as choosing 'yes, I'm sure I want to
set existing rows to emptystring' would be equivalent to choosing 'set a
one-time default' and entering emptystring manually. But the difference
would be that the user doesn't feel like they've done something wrong when
they see this. I can say from experience, whenever I saw the questioner in
the past, I quickly cancelled it and went back to try and fix up my models,
assuming I'd made a mistake.
Cheers,
Jarek
On Tuesday, September 13, 2016 at 9:13:11 PM UTC+10, Markus Holtermann
wrote:
>
> Thank you for your input, Jarek.
>
> Assuming I have an existing model, adding
>
> models.CharField(blank=True, max_length=150)
>
> to it, doesn't invoke the questioner on current master. Changing this
> field to
>
> models.CharField(max_length=150)
>
> doesn't call the questioner either.
>
> Looking at the SQL Django generates, the first change results in
>
> ALTER TABLE "example_mymodel" ADD COLUMN "name2" varchar(150) DEFAULT
> '' NOT NULL;
> ALTER TABLE "example_mymodel" ALTER COLUMN "name2" DROP DEFAULT;
>
> and the second in a noop.
>
> I'm trying to understand what change you're proposing in order to figure
> out if going forward with your proposal is something we should do.
>
> Cheers,
>
> Markus
>
> On Sun, Sep 11, 2016 at 05:08:38AM -0700, Jarek Glowacki wrote:
> >Mm, convenience over strict correctness. Perhaps all that's needed is a
> >slight rephrasing of the prompt and we can have both?
> >
> >Adding field with no `blank=True` and no `null=True`:
> >
> >You are trying to add a non-nullable field '%s' to %s without a default;
> we
> >can't do that (the database needs something to populate existing rows).
> >[provide default | cancel]
> >
> >Removing `null=True` from field:
> >
> >You are trying to change the nullable field '%s' on %s to non-nullable
> >without a default; we can't do that (the database needs something to
> >populate existing rows). [provide default | fix later | cancel]
> >
> >Removing `blank` from field:
> >
> >?? (I only skimmed the code; not sure if this even opens up questioner)
> >
> >
> >This kind of implies to the user that they're doing something wrong.
> Maybe
> >if, for string fields, it read something more along the lines of:
> >
> >You are adding/altering a field without setting `blank=True`. This will
> >populate existing rows with emptystring despite it being an invalid form
> >input. Are you sure? [yes | let me provide a one-time default for
> existing
> >rows | cancel]
> >
> >
> >So basically changing "you can't do this!" (exception-esque) to "are you
> >sure you want this?" (warning-esque).
> >
> >Anyway I think I'll leave it here. I've exhausted my discussion points
> now,
> >and you already resolved my particular use case back in the ticket, so I
> no
> >longer feel so strongly about this to continue trying to push for a
> change
> >(though am willing to submit a PR if any of the suggested changes are
> >approved).
> >
> >Cheers,
> >
> >
> >On Saturday, September 10, 2016 at 10:08:27 AM UTC+10, Tim Graham wrote:
> >>
> >> Sure, but I don't think that use case should take priority. It's not
> much
> >> work to type an empty string into the questioner if that's what you
> want.
> >> If we remove the prompt, it's significantly more work (editing a
> migration
> >> file or using RunPython) for a developer to set a non-empty value.
> >>
> >> On Friday, September 9, 2016 at 7:19:37 PM UTC-4, Jarek Glowacki wrote:
> >>>
> >>> Instances created afterwards, via `MyModel.objects.create()`, with
> this
> >>> field unset won't pass form validation either.
> >>> The use case is where this field is not expected to appear on a Django
> >>> form.
> >>>
> >>> On Friday, September 9, 2016 at 11:58:38 PM UTC+10, Tim Graham wrote:
> >>>>
> >>>> If blank=False, then a new column with a non-blank value means that
> all
> >>>> existing objects won't pass form validation. Therefore, I don't see
> why a
> >>>> prompt for a value isn't helpful.
> >>>>
> >>>> On Friday, September 9, 2016 at 6:42:02 AM UTC-4, Jarek Glowacki
> wrote:
> >>>>>
> >>>>> I made a rant/ticket regarding the hidden usage of `blank` here:
> #27197
> >>>>> <https://code.djangoproject.com/ticket/27197#ticket>.
> >>>>>
> >>>>> In short, I don't think that `blank` should dictate whether or not
> the
> >>>>> migration questioner runs.
> >>>>> Building on this, I don't think it should run for for string-type
> >>>>> fields at all. If they have `default` set, use that for existing
> rows. Else
> >>>>> if they have `null=True`, set existing rows to `NULL`. Else, set
> existing
> >>>>> rows to empty string.
> >>>>>
> >>>>> See linked rant/ticket for some (hopefully) compelling arguments..
> >>>>>
> >>>>> Thoughts?
> >>>>>
> >>>>
> >
> >--
> >You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> >To unsubscribe from this group and stop receiving emails from it, send an
> email to django-develop...@googlegroups.com <javascript:>.
> >To post to this group, send email to django-d...@googlegroups.com
> <javascript:>.
> >Visit this group at https://groups.google.com/group/django-developers.
> >To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/60bf1464-7e70-41c4-b8de-b201af84b4b7%40googlegroups.com.
>
>
> >For more options, visit https://groups.google.com/d/optout.
>
>
--
You received this message because you are subscribed to the Google Groups
"Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-developers/107dce69-5be0-489a-9801-d670c2268c0d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
