Re: Requiring sqlparse for sqlite introspection

[Tim Graham]

I created a ticket and a pull request to add sqlparse as a required 
dependency.
https://code.djangoproject.com/ticket/29934
https://github.com/django/django/pull/10622

On Saturday, November 3, 2018 at 4:47:30 PM UTC-4, charettes wrote:
>
> > So you want to add it to Django's install_requires even though it won't 
> be necessary if SQLite isn't used?
>
> That's my understanding and what I was advocating for.
>
> Simon
>
> Le samedi 3 novembre 2018 10:09:55 UTC-4, Tim Graham a écrit :
>>
>> So you want to add it to Django's install_requires even though it won't 
>> be necessary if SQLite isn't used? It seems okay to me. It's an extra 40k 
>> or so of disk space but that's not much compared to all the extra stuff 
>> Django comes with that every Django project doesn't necessarily use. We 
>> also have the requires_sqlparse_for_splitting feature flag which we could 
>> remove if we can assume sqlparse is installed.
>>
>> On Sunday, October 28, 2018 at 4:59:54 AM UTC-4, Adam Johnson wrote:
>>>
>>> I'm fine with adding it as a dependency, my experience has been that 
>>> it's a stable, well-maintained package over the past few years I've used it.
>>>
>>> On Sun, 28 Oct 2018 at 05:00, charettes  wrote:
>>>
 After a bit of work to minimize the cases where sqlparse would be a 
 required at runtime for SQLite to AddConstraint/RemoveConstraint 
 operations 
 [0] I came to the conclusion that it would make more sense to make 
 sqlparse 
 an hard dependency of Django 2.2.

 The two reasons backing this conclusions are

 1. Given we run the suite with sqlparse installed on CI it will be 
 really hard to prevent inadvertently breaking the promise of a soft 
 dependency on sqlparse for Meta.constraints only. I guess we could 
 have a daily CI job but that would quickly get out of hand once we have to 
 perform backport and such.

 2. There's a few instances of fragile regex parsing that could be made 
 more reliable if sqlparse was an hard dependency for SQLite. Two 
 examples are
  - 
 https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/introspection.py#L90-L146
  - 
 https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/schema.py#L91-L127

 Given the transition to require pytz in Django 1.11 went smoothly and 
 our needs for sqlparse are similar to provide a solid experience on SQLite 
 I'd be +1 on requiring it from Django 2.2 to a point where the lowest 
 version of SQLite we support has better introspection capabilities.

 Simon

 [0] https://github.com/django/django/pull/10572

 Le mercredi 10 octobre 2018 14:53:53 UTC-4, Tim Graham a écrit :
>
> sqlparse is already installed as part of Django's tests. The question 
> is whether sqlparse should be mandatory for SQLite users (i.e. when 
> getting 
> started with a new project an error message will say, "You must install 
> sqlparse to use SQLite" (I don't think there's a way to install it 
> automatically unless we install it for all users, regardless of database) 
> or if we should try to make it optional and only raise an error if a 
> project's migrations require it.
>
> The question is "what percentage of SQLite projects are going to end 
> up needing sqlparse based on their migrations? If it's high enough, it 
> might make for a better user experience to have users install it when 
> they're getting started instead of a lazy error when get_constraints() 
> is called."
>
> On Monday, October 8, 2018 at 3:22:32 AM UTC-4, Andrew Godwin wrote:
>>
>> Adding sqlparse into the introspection code for SQLite specifically 
>> means it's going to be a runtime dependency for anything to do with 
>> migrations.
>>
>> I'm alright having that be the case, but I do think we should make 
>> sure the tests run with SQLite as otherwise a large section of the most 
>> complicated code in migrations won't be tested properly.
>>
>> Andrew
>>
>>
>> On Mon, 8 Oct 2018, 00:59 Ian Foote,  wrote:
>>
>>> Hi all,
>>>
>>> On my pull request (https://github.com/django/django/pull/10406) 
>>> refactoring how Django creates database constraints I introduced a 
>>> dependency on sqlparse in the sqlite introspection code. This allows 
>>> Django 
>>> to correctly read information about constraints on sqlite, particularly 
>>> the 
>>> name.
>>>
>>> When reviewing (
>>> https://github.com/django/django/pull/10406#issuecomment-424542217) 
>>> Tim Graham raised the question of whether we should make sqlparse a 
>>> mandatory requirement for the sqlite tests or if we should skip those 
>>> tests 
>>> that require it. In later discussion (
>>> https://github.com/django/django/p

Re: Requiring sqlparse for sqlite introspection

[Adam Johnson]

If you can see a way to improve those docs submit a PR

On Sun, 4 Nov 2018 at 01:51, Dan Davis  wrote:

> I just joined as a contributor, but I've shipped an appliance install
> running using rpms, anaconda (the other one), and pungi.   Depending on
> sqlparse doesn't seem to me a big deal.  It already gets invoked for me
> during migrations.  I cannot recall what caused it to be installed. One
> thing we should definitely do however, is more clearly document that RunSQL
> migrations will split your SQL on statement boundaries to the extent it can
> unless you run manual SQL migrations like this:
>
> RunSQL([forward_compound_statement], [backward_compound_statement])
>
>
> Creating functions with Oracle doesn't work without this.
>
> On Saturday, November 3, 2018 at 4:47:30 PM UTC-4, charettes wrote:
>>
>> > So you want to add it to Django's install_requires even though it won't
>> be necessary if SQLite isn't used?
>>
>> That's my understanding and what I was advocating for.
>>
>> Simonon
>>
>
>> Le samedi 3 novembre 2018 10:09:55 UTC-4, Tim Graham a écrit :
>>>
>>> So you want to add it to Django's install_requires even though it won't
>>> be necessary if SQLite isn't used? It seems okay to me. It's an extra 40k
>>> or so of disk space but that's not much compared to all the extra stuff
>>> Django comes with that every Django project doesn't necessarily use. We
>>> also have the requires_sqlparse_for_splitting feature flag which we could
>>> remove if we can assume sqlparse is installed.
>>>
>>> On Sunday, October 28, 2018 at 4:59:54 AM UTC-4, Adam Johnson wrote:

 I'm fine with adding it as a dependency, my experience has been that
 it's a stable, well-maintained package over the past few years I've used 
 it.

 On Sun, 28 Oct 2018 at 05:00, charettes  wrote:

> After a bit of work to minimize the cases where sqlparse would be a
> required at runtime for SQLite to AddConstraint/RemoveConstraint 
> operations
> [0] I came to the conclusion that it would make more sense to make 
> sqlparse
> an hard dependency of Django 2.2.
>
> The two reasons backing this conclusions are
>
> 1. Given we run the suite with sqlparse installed on CI it will be
> really hard to prevent inadvertently breaking the promise of a soft
> dependency on sqlparse for Meta.constraints only. I guess we could
> have a daily CI job but that would quickly get out of hand once we have to
> perform backport and such.
>
> 2. There's a few instances of fragile regex parsing that could be made
> more reliable if sqlparse was an hard dependency for SQLite. Two
> examples are
>  -
> https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/introspection.py#L90-L146
>  -
> https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/schema.py#L91-L127
>
> Given the transition to require pytz in Django 1.11 went smoothly and
> our needs for sqlparse are similar to provide a solid experience on SQLite
> I'd be +1 on requiring it from Django 2.2 to a point where the lowest
> version of SQLite we support has better introspection capabilities.
>
> Simon
>
> [0] https://github.com/django/django/pull/10572
>
> Le mercredi 10 octobre 2018 14:53:53 UTC-4, Tim Graham a écrit :
>>
>> sqlparse is already installed as part of Django's tests. The question
>> is whether sqlparse should be mandatory for SQLite users (i.e. when 
>> getting
>> started with a new project an error message will say, "You must install
>> sqlparse to use SQLite" (I don't think there's a way to install it
>> automatically unless we install it for all users, regardless of database)
>> or if we should try to make it optional and only raise an error if a
>> project's migrations require it.
>>
>> The question is "what percentage of SQLite projects are going to end
>> up needing sqlparse based on their migrations? If it's high enough, it
>> might make for a better user experience to have users install it when
>> they're getting started instead of a lazy error when
>> get_constraints() is called."
>>
>> On Monday, October 8, 2018 at 3:22:32 AM UTC-4, Andrew Godwin wrote:
>>>
>>> Adding sqlparse into the introspection code for SQLite specifically
>>> means it's going to be a runtime dependency for anything to do with
>>> migrations.
>>>
>>> I'm alright having that be the case, but I do think we should make
>>> sure the tests run with SQLite as otherwise a large section of the most
>>> complicated code in migrations won't be tested properly.
>>>
>>> Andrew
>>>
>>>
>>> On Mon, 8 Oct 2018, 00:59 Ian Foote,  wrote:
>>>
 Hi all,

 On my pull request (https://github.com/django/django/pull/10406)
 

Re: Requiring sqlparse for sqlite introspection

[Dan Davis]

I just joined as a contributor, but I've shipped an appliance install 
running using rpms, anaconda (the other one), and pungi.   Depending on 
sqlparse doesn't seem to me a big deal.  It already gets invoked for me 
during migrations.  I cannot recall what caused it to be installed. One 
thing we should definitely do however, is more clearly document that RunSQL 
migrations will split your SQL on statement boundaries to the extent it can 
unless you run manual SQL migrations like this:

RunSQL([forward_compound_statement], [backward_compound_statement])


Creating functions with Oracle doesn't work without this.

On Saturday, November 3, 2018 at 4:47:30 PM UTC-4, charettes wrote:
>
> > So you want to add it to Django's install_requires even though it won't 
> be necessary if SQLite isn't used?
>
> That's my understanding and what I was advocating for.
>
> Simonon 
>
> Le samedi 3 novembre 2018 10:09:55 UTC-4, Tim Graham a écrit :
>>
>> So you want to add it to Django's install_requires even though it won't 
>> be necessary if SQLite isn't used? It seems okay to me. It's an extra 40k 
>> or so of disk space but that's not much compared to all the extra stuff 
>> Django comes with that every Django project doesn't necessarily use. We 
>> also have the requires_sqlparse_for_splitting feature flag which we could 
>> remove if we can assume sqlparse is installed.
>>
>> On Sunday, October 28, 2018 at 4:59:54 AM UTC-4, Adam Johnson wrote:
>>>
>>> I'm fine with adding it as a dependency, my experience has been that 
>>> it's a stable, well-maintained package over the past few years I've used it.
>>>
>>> On Sun, 28 Oct 2018 at 05:00, charettes  wrote:
>>>
 After a bit of work to minimize the cases where sqlparse would be a 
 required at runtime for SQLite to AddConstraint/RemoveConstraint 
 operations 
 [0] I came to the conclusion that it would make more sense to make 
 sqlparse 
 an hard dependency of Django 2.2.

 The two reasons backing this conclusions are

 1. Given we run the suite with sqlparse installed on CI it will be 
 really hard to prevent inadvertently breaking the promise of a soft 
 dependency on sqlparse for Meta.constraints only. I guess we could 
 have a daily CI job but that would quickly get out of hand once we have to 
 perform backport and such.

 2. There's a few instances of fragile regex parsing that could be made 
 more reliable if sqlparse was an hard dependency for SQLite. Two 
 examples are
  - 
 https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/introspection.py#L90-L146
  - 
 https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/schema.py#L91-L127

 Given the transition to require pytz in Django 1.11 went smoothly and 
 our needs for sqlparse are similar to provide a solid experience on SQLite 
 I'd be +1 on requiring it from Django 2.2 to a point where the lowest 
 version of SQLite we support has better introspection capabilities.

 Simon

 [0] https://github.com/django/django/pull/10572

 Le mercredi 10 octobre 2018 14:53:53 UTC-4, Tim Graham a écrit :
>
> sqlparse is already installed as part of Django's tests. The question 
> is whether sqlparse should be mandatory for SQLite users (i.e. when 
> getting 
> started with a new project an error message will say, "You must install 
> sqlparse to use SQLite" (I don't think there's a way to install it 
> automatically unless we install it for all users, regardless of database) 
> or if we should try to make it optional and only raise an error if a 
> project's migrations require it.
>
> The question is "what percentage of SQLite projects are going to end 
> up needing sqlparse based on their migrations? If it's high enough, it 
> might make for a better user experience to have users install it when 
> they're getting started instead of a lazy error when get_constraints() 
> is called."
>
> On Monday, October 8, 2018 at 3:22:32 AM UTC-4, Andrew Godwin wrote:
>>
>> Adding sqlparse into the introspection code for SQLite specifically 
>> means it's going to be a runtime dependency for anything to do with 
>> migrations.
>>
>> I'm alright having that be the case, but I do think we should make 
>> sure the tests run with SQLite as otherwise a large section of the most 
>> complicated code in migrations won't be tested properly.
>>
>> Andrew
>>
>>
>> On Mon, 8 Oct 2018, 00:59 Ian Foote,  wrote:
>>
>>> Hi all,
>>>
>>> On my pull request (https://github.com/django/django/pull/10406) 
>>> refactoring how Django creates database constraints I introduced a 
>>> dependency on sqlparse in the sqlite introspection code. This allows 
>>> Django 
>>> to correctly re

Re: Requiring sqlparse for sqlite introspection

[charettes]

> So you want to add it to Django's install_requires even though it won't 
be necessary if SQLite isn't used?

That's my understanding and what I was advocating for.

Simon

Le samedi 3 novembre 2018 10:09:55 UTC-4, Tim Graham a écrit :
>
> So you want to add it to Django's install_requires even though it won't be 
> necessary if SQLite isn't used? It seems okay to me. It's an extra 40k or 
> so of disk space but that's not much compared to all the extra stuff Django 
> comes with that every Django project doesn't necessarily use. We also have 
> the requires_sqlparse_for_splitting feature flag which we could remove if 
> we can assume sqlparse is installed.
>
> On Sunday, October 28, 2018 at 4:59:54 AM UTC-4, Adam Johnson wrote:
>>
>> I'm fine with adding it as a dependency, my experience has been that it's 
>> a stable, well-maintained package over the past few years I've used it.
>>
>> On Sun, 28 Oct 2018 at 05:00, charettes  wrote:
>>
>>> After a bit of work to minimize the cases where sqlparse would be a 
>>> required at runtime for SQLite to AddConstraint/RemoveConstraint operations 
>>> [0] I came to the conclusion that it would make more sense to make sqlparse 
>>> an hard dependency of Django 2.2.
>>>
>>> The two reasons backing this conclusions are
>>>
>>> 1. Given we run the suite with sqlparse installed on CI it will be 
>>> really hard to prevent inadvertently breaking the promise of a soft 
>>> dependency on sqlparse for Meta.constraints only. I guess we could have 
>>> a daily CI job but that would quickly get out of hand once we have to 
>>> perform backport and such.
>>>
>>> 2. There's a few instances of fragile regex parsing that could be made 
>>> more reliable if sqlparse was an hard dependency for SQLite. Two 
>>> examples are
>>>  - 
>>> https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/introspection.py#L90-L146
>>>  - 
>>> https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/schema.py#L91-L127
>>>
>>> Given the transition to require pytz in Django 1.11 went smoothly and 
>>> our needs for sqlparse are similar to provide a solid experience on SQLite 
>>> I'd be +1 on requiring it from Django 2.2 to a point where the lowest 
>>> version of SQLite we support has better introspection capabilities.
>>>
>>> Simon
>>>
>>> [0] https://github.com/django/django/pull/10572
>>>
>>> Le mercredi 10 octobre 2018 14:53:53 UTC-4, Tim Graham a écrit :

 sqlparse is already installed as part of Django's tests. The question 
 is whether sqlparse should be mandatory for SQLite users (i.e. when 
 getting 
 started with a new project an error message will say, "You must install 
 sqlparse to use SQLite" (I don't think there's a way to install it 
 automatically unless we install it for all users, regardless of database) 
 or if we should try to make it optional and only raise an error if a 
 project's migrations require it.

 The question is "what percentage of SQLite projects are going to end up 
 needing sqlparse based on their migrations? If it's high enough, it might 
 make for a better user experience to have users install it when they're 
 getting started instead of a lazy error when get_constraints() is 
 called."

 On Monday, October 8, 2018 at 3:22:32 AM UTC-4, Andrew Godwin wrote:
>
> Adding sqlparse into the introspection code for SQLite specifically 
> means it's going to be a runtime dependency for anything to do with 
> migrations.
>
> I'm alright having that be the case, but I do think we should make 
> sure the tests run with SQLite as otherwise a large section of the most 
> complicated code in migrations won't be tested properly.
>
> Andrew
>
>
> On Mon, 8 Oct 2018, 00:59 Ian Foote,  wrote:
>
>> Hi all,
>>
>> On my pull request (https://github.com/django/django/pull/10406) 
>> refactoring how Django creates database constraints I introduced a 
>> dependency on sqlparse in the sqlite introspection code. This allows 
>> Django 
>> to correctly read information about constraints on sqlite, particularly 
>> the 
>> name.
>>
>> When reviewing (
>> https://github.com/django/django/pull/10406#issuecomment-424542217) 
>> Tim Graham raised the question of whether we should make sqlparse a 
>> mandatory requirement for the sqlite tests or if we should skip those 
>> tests 
>> that require it. In later discussion (
>> https://github.com/django/django/pull/10406#issuecomment-427362983), 
>> Tim raised the point that the introspection code is used by migrations.
>>
>> I'm not clear myself what the best route forwards here is, so I'm 
>> bringing it to Django Developers for discussion (as Tim suggested). 
>>
>> Thanks,
>> Ian
>>
>> -- 
>> You received this message bec

Re: Requiring sqlparse for sqlite introspection

[Tim Graham]

So you want to add it to Django's install_requires even though it won't be 
necessary if SQLite isn't used? It seems okay to me. It's an extra 40k or 
so of disk space but that's not much compared to all the extra stuff Django 
comes with that every Django project doesn't necessarily use. We also have 
the requires_sqlparse_for_splitting feature flag which we could remove if 
we can assume sqlparse is installed.

On Sunday, October 28, 2018 at 4:59:54 AM UTC-4, Adam Johnson wrote:
>
> I'm fine with adding it as a dependency, my experience has been that it's 
> a stable, well-maintained package over the past few years I've used it.
>
> On Sun, 28 Oct 2018 at 05:00, charettes > 
> wrote:
>
>> After a bit of work to minimize the cases where sqlparse would be a 
>> required at runtime for SQLite to AddConstraint/RemoveConstraint operations 
>> [0] I came to the conclusion that it would make more sense to make sqlparse 
>> an hard dependency of Django 2.2.
>>
>> The two reasons backing this conclusions are
>>
>> 1. Given we run the suite with sqlparse installed on CI it will be 
>> really hard to prevent inadvertently breaking the promise of a soft 
>> dependency on sqlparse for Meta.constraints only. I guess we could have 
>> a daily CI job but that would quickly get out of hand once we have to 
>> perform backport and such.
>>
>> 2. There's a few instances of fragile regex parsing that could be made 
>> more reliable if sqlparse was an hard dependency for SQLite. Two 
>> examples are
>>  - 
>> https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/introspection.py#L90-L146
>>  - 
>> https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/schema.py#L91-L127
>>
>> Given the transition to require pytz in Django 1.11 went smoothly and our 
>> needs for sqlparse are similar to provide a solid experience on SQLite I'd 
>> be +1 on requiring it from Django 2.2 to a point where the lowest version 
>> of SQLite we support has better introspection capabilities.
>>
>> Simon
>>
>> [0] https://github.com/django/django/pull/10572
>>
>> Le mercredi 10 octobre 2018 14:53:53 UTC-4, Tim Graham a écrit :
>>>
>>> sqlparse is already installed as part of Django's tests. The question is 
>>> whether sqlparse should be mandatory for SQLite users (i.e. when getting 
>>> started with a new project an error message will say, "You must install 
>>> sqlparse to use SQLite" (I don't think there's a way to install it 
>>> automatically unless we install it for all users, regardless of database) 
>>> or if we should try to make it optional and only raise an error if a 
>>> project's migrations require it.
>>>
>>> The question is "what percentage of SQLite projects are going to end up 
>>> needing sqlparse based on their migrations? If it's high enough, it might 
>>> make for a better user experience to have users install it when they're 
>>> getting started instead of a lazy error when get_constraints() is 
>>> called."
>>>
>>> On Monday, October 8, 2018 at 3:22:32 AM UTC-4, Andrew Godwin wrote:

 Adding sqlparse into the introspection code for SQLite specifically 
 means it's going to be a runtime dependency for anything to do with 
 migrations.

 I'm alright having that be the case, but I do think we should make sure 
 the tests run with SQLite as otherwise a large section of the most 
 complicated code in migrations won't be tested properly.

 Andrew


 On Mon, 8 Oct 2018, 00:59 Ian Foote,  wrote:

> Hi all,
>
> On my pull request (https://github.com/django/django/pull/10406) 
> refactoring how Django creates database constraints I introduced a 
> dependency on sqlparse in the sqlite introspection code. This allows 
> Django 
> to correctly read information about constraints on sqlite, particularly 
> the 
> name.
>
> When reviewing (
> https://github.com/django/django/pull/10406#issuecomment-424542217) 
> Tim Graham raised the question of whether we should make sqlparse a 
> mandatory requirement for the sqlite tests or if we should skip those 
> tests 
> that require it. In later discussion (
> https://github.com/django/django/pull/10406#issuecomment-427362983), 
> Tim raised the point that the introspection code is used by migrations.
>
> I'm not clear myself what the best route forwards here is, so I'm 
> bringing it to Django Developers for discussion (as Tim suggested). 
>
> Thanks,
> Ian
>
> -- 
> 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.
> To post to this group, send email to django-d...@googlegroups.com.
> Visit this group at https:/

Re: Requiring sqlparse for sqlite introspection

[Adam Johnson]

I'm fine with adding it as a dependency, my experience has been that it's a
stable, well-maintained package over the past few years I've used it.

On Sun, 28 Oct 2018 at 05:00, charettes  wrote:

> After a bit of work to minimize the cases where sqlparse would be a
> required at runtime for SQLite to AddConstraint/RemoveConstraint operations
> [0] I came to the conclusion that it would make more sense to make sqlparse
> an hard dependency of Django 2.2.
>
> The two reasons backing this conclusions are
>
> 1. Given we run the suite with sqlparse installed on CI it will be really
> hard to prevent inadvertently breaking the promise of a soft dependency on
> sqlparse for Meta.constraints only. I guess we could have a daily CI job
> but that would quickly get out of hand once we have to perform backport and
> such.
>
> 2. There's a few instances of fragile regex parsing that could be made
> more reliable if sqlparse was an hard dependency for SQLite. Two examples
> are
>  -
> https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/introspection.py#L90-L146
>  -
> https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/schema.py#L91-L127
>
> Given the transition to require pytz in Django 1.11 went smoothly and our
> needs for sqlparse are similar to provide a solid experience on SQLite I'd
> be +1 on requiring it from Django 2.2 to a point where the lowest version
> of SQLite we support has better introspection capabilities.
>
> Simon
>
> [0] https://github.com/django/django/pull/10572
>
> Le mercredi 10 octobre 2018 14:53:53 UTC-4, Tim Graham a écrit :
>>
>> sqlparse is already installed as part of Django's tests. The question is
>> whether sqlparse should be mandatory for SQLite users (i.e. when getting
>> started with a new project an error message will say, "You must install
>> sqlparse to use SQLite" (I don't think there's a way to install it
>> automatically unless we install it for all users, regardless of database)
>> or if we should try to make it optional and only raise an error if a
>> project's migrations require it.
>>
>> The question is "what percentage of SQLite projects are going to end up
>> needing sqlparse based on their migrations? If it's high enough, it might
>> make for a better user experience to have users install it when they're
>> getting started instead of a lazy error when get_constraints() is
>> called."
>>
>> On Monday, October 8, 2018 at 3:22:32 AM UTC-4, Andrew Godwin wrote:
>>>
>>> Adding sqlparse into the introspection code for SQLite specifically
>>> means it's going to be a runtime dependency for anything to do with
>>> migrations.
>>>
>>> I'm alright having that be the case, but I do think we should make sure
>>> the tests run with SQLite as otherwise a large section of the most
>>> complicated code in migrations won't be tested properly.
>>>
>>> Andrew
>>>
>>>
>>> On Mon, 8 Oct 2018, 00:59 Ian Foote,  wrote:
>>>
 Hi all,

 On my pull request (https://github.com/django/django/pull/10406)
 refactoring how Django creates database constraints I introduced a
 dependency on sqlparse in the sqlite introspection code. This allows Django
 to correctly read information about constraints on sqlite, particularly the
 name.

 When reviewing (
 https://github.com/django/django/pull/10406#issuecomment-424542217)
 Tim Graham raised the question of whether we should make sqlparse a
 mandatory requirement for the sqlite tests or if we should skip those tests
 that require it. In later discussion (
 https://github.com/django/django/pull/10406#issuecomment-427362983),
 Tim raised the point that the introspection code is used by migrations.

 I'm not clear myself what the best route forwards here is, so I'm
 bringing it to Django Developers for discussion (as Tim suggested).

 Thanks,
 Ian

 --
 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.
 To post to this group, send email to django-d...@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/CAFv-zfKadOeWit8M6GMmx4H2ChUCU6u%3DscHX8F7oBKJkHRbuVg%40mail.gmail.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 receiv

Re: Requiring sqlparse for sqlite introspection

[charettes]

After a bit of work to minimize the cases where sqlparse would be a 
required at runtime for SQLite to AddConstraint/RemoveConstraint operations 
[0] I came to the conclusion that it would make more sense to make sqlparse 
an hard dependency of Django 2.2.

The two reasons backing this conclusions are

1. Given we run the suite with sqlparse installed on CI it will be really 
hard to prevent inadvertently breaking the promise of a soft dependency on 
sqlparse for Meta.constraints only. I guess we could have a daily CI job 
but that would quickly get out of hand once we have to perform backport and 
such.

2. There's a few instances of fragile regex parsing that could be made more 
reliable if sqlparse was an hard dependency for SQLite. Two examples are
 - 
https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/introspection.py#L90-L146
 - 
https://github.com/django/django/blob/f892781b957f674806a227a10c58768f66a48c07/django/db/backends/sqlite3/schema.py#L91-L127

Given the transition to require pytz in Django 1.11 went smoothly and our 
needs for sqlparse are similar to provide a solid experience on SQLite I'd 
be +1 on requiring it from Django 2.2 to a point where the lowest version 
of SQLite we support has better introspection capabilities.

Simon

[0] https://github.com/django/django/pull/10572

Le mercredi 10 octobre 2018 14:53:53 UTC-4, Tim Graham a écrit :
>
> sqlparse is already installed as part of Django's tests. The question is 
> whether sqlparse should be mandatory for SQLite users (i.e. when getting 
> started with a new project an error message will say, "You must install 
> sqlparse to use SQLite" (I don't think there's a way to install it 
> automatically unless we install it for all users, regardless of database) 
> or if we should try to make it optional and only raise an error if a 
> project's migrations require it.
>
> The question is "what percentage of SQLite projects are going to end up 
> needing sqlparse based on their migrations? If it's high enough, it might 
> make for a better user experience to have users install it when they're 
> getting started instead of a lazy error when get_constraints() is called."
>
> On Monday, October 8, 2018 at 3:22:32 AM UTC-4, Andrew Godwin wrote:
>>
>> Adding sqlparse into the introspection code for SQLite specifically means 
>> it's going to be a runtime dependency for anything to do with migrations.
>>
>> I'm alright having that be the case, but I do think we should make sure 
>> the tests run with SQLite as otherwise a large section of the most 
>> complicated code in migrations won't be tested properly.
>>
>> Andrew
>>
>>
>> On Mon, 8 Oct 2018, 00:59 Ian Foote,  wrote:
>>
>>> Hi all,
>>>
>>> On my pull request (https://github.com/django/django/pull/10406) 
>>> refactoring how Django creates database constraints I introduced a 
>>> dependency on sqlparse in the sqlite introspection code. This allows Django 
>>> to correctly read information about constraints on sqlite, particularly the 
>>> name.
>>>
>>> When reviewing (
>>> https://github.com/django/django/pull/10406#issuecomment-424542217) Tim 
>>> Graham raised the question of whether we should make sqlparse a mandatory 
>>> requirement for the sqlite tests or if we should skip those tests that 
>>> require it. In later discussion (
>>> https://github.com/django/django/pull/10406#issuecomment-427362983), 
>>> Tim raised the point that the introspection code is used by migrations.
>>>
>>> I'm not clear myself what the best route forwards here is, so I'm 
>>> bringing it to Django Developers for discussion (as Tim suggested). 
>>>
>>> Thanks,
>>> Ian
>>>
>>> -- 
>>> 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.
>>> To post to this group, send email to django-d...@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/CAFv-zfKadOeWit8M6GMmx4H2ChUCU6u%3DscHX8F7oBKJkHRbuVg%40mail.gmail.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:/

Re: Requiring sqlparse for sqlite introspection

[Tim Graham]

sqlparse is already installed as part of Django's tests. The question is 
whether sqlparse should be mandatory for SQLite users (i.e. when getting 
started with a new project an error message will say, "You must install 
sqlparse to use SQLite" (I don't think there's a way to install it 
automatically unless we install it for all users, regardless of database) 
or if we should try to make it optional and only raise an error if a 
project's migrations require it.

The question is "what percentage of SQLite projects are going to end up 
needing sqlparse based on their migrations? If it's high enough, it might 
make for a better user experience to have users install it when they're 
getting started instead of a lazy error when get_constraints() is called."

On Monday, October 8, 2018 at 3:22:32 AM UTC-4, Andrew Godwin wrote:
>
> Adding sqlparse into the introspection code for SQLite specifically means 
> it's going to be a runtime dependency for anything to do with migrations.
>
> I'm alright having that be the case, but I do think we should make sure 
> the tests run with SQLite as otherwise a large section of the most 
> complicated code in migrations won't be tested properly.
>
> Andrew
>
>
> On Mon, 8 Oct 2018, 00:59 Ian Foote, > wrote:
>
>> Hi all,
>>
>> On my pull request (https://github.com/django/django/pull/10406) 
>> refactoring how Django creates database constraints I introduced a 
>> dependency on sqlparse in the sqlite introspection code. This allows Django 
>> to correctly read information about constraints on sqlite, particularly the 
>> name.
>>
>> When reviewing (
>> https://github.com/django/django/pull/10406#issuecomment-424542217) Tim 
>> Graham raised the question of whether we should make sqlparse a mandatory 
>> requirement for the sqlite tests or if we should skip those tests that 
>> require it. In later discussion (
>> https://github.com/django/django/pull/10406#issuecomment-427362983), Tim 
>> raised the point that the introspection code is used by migrations.
>>
>> I'm not clear myself what the best route forwards here is, so I'm 
>> bringing it to Django Developers for discussion (as Tim suggested). 
>>
>> Thanks,
>> Ian
>>
>> -- 
>> 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 .
>> To post to this group, send email to django-d...@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/CAFv-zfKadOeWit8M6GMmx4H2ChUCU6u%3DscHX8F7oBKJkHRbuVg%40mail.gmail.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/9a5822a0-18c7-4fef-b1d6-b09e9bb63805%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Requiring sqlparse for sqlite introspection

[Andrew Godwin]

Adding sqlparse into the introspection code for SQLite specifically means
it's going to be a runtime dependency for anything to do with migrations.

I'm alright having that be the case, but I do think we should make sure the
tests run with SQLite as otherwise a large section of the most complicated
code in migrations won't be tested properly.

Andrew


On Mon, 8 Oct 2018, 00:59 Ian Foote,  wrote:

> Hi all,
>
> On my pull request (https://github.com/django/django/pull/10406)
> refactoring how Django creates database constraints I introduced a
> dependency on sqlparse in the sqlite introspection code. This allows Django
> to correctly read information about constraints on sqlite, particularly the
> name.
>
> When reviewing (
> https://github.com/django/django/pull/10406#issuecomment-424542217) Tim
> Graham raised the question of whether we should make sqlparse a mandatory
> requirement for the sqlite tests or if we should skip those tests that
> require it. In later discussion (
> https://github.com/django/django/pull/10406#issuecomment-427362983), Tim
> raised the point that the introspection code is used by migrations.
>
> I'm not clear myself what the best route forwards here is, so I'm bringing
> it to Django Developers for discussion (as Tim suggested).
>
> Thanks,
> Ian
>
> --
> 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/CAFv-zfKadOeWit8M6GMmx4H2ChUCU6u%3DscHX8F7oBKJkHRbuVg%40mail.gmail.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/CAFwN1upormR5UjCNMU7xXQiqeLtU%2BP-qBQAdgLx%2Be2Jno%2ByBew%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.