Re: Stop QuerySet repr from executing queries

2021-03-23 Thread Joachim Jablon
Just been bitten by that same bug (combination of Sentry, using a 
Queryset.as_manager() that creates an unfiltered queryset as a local 
variable in the stack, a create signal that errored provoking a stacktrace 
that contained the queryset, a table that is always filtered by a field, 
and sorted by another field and an index that behaves poorly without the 
aformentionned filter).

So would a contribution that would avoid evaluating the uncached queryset 
on repr when DEBUG is False likely to be accepted ? If so, I'm ready to get 
my hands dirty :)

Cheers !

Le dimanche 7 mars 2021 à 00:54:56 UTC+1, mic...@turbosys.com.br a écrit :

> I agree with Matt on that. Avoiding executing the queries on production 
> would be helpful!
>
> Let me share my case. I use django-rest-framework in a specific project 
> and DRF has a feature: a serializer has a fancier string representation 
> when printed. I was using a serializer with a queryset in a view that had 
> an exception further, which caused the serializer to be logged and thus, 
> the queryset to be executed.
>
> There are more details in this discussion: 
> https://github.com/encode/django-rest-framework/discussions/7782
>
> The short story is: I was logging this serializer passively and it was 
> causing the execution of a query to a whole table with millions of records, 
> without any sorting optimization, creating hard to debug performance 
> problems.
>
> I understand it is an unusual situation, but repeating Matt's words: 
> Django should be reliable in production for any combination of those 
> conditions. 
>
> Thanks!
>
> On Sunday, October 20, 2019 at 4:47:25 PM UTC-3 Matt wrote:
>
>> Perhaps we ought to just keep the current behavior when DEBUG is True (it 
>> seems so obvious now, I can't believe it wasn't the first thing I 
>> suggested)? Django does lots of helpful things in DEBUG mode at the expense 
>> of performance. I think this would be an innocuous change for most people. 
>>
>> It is not the most important thing to remove behavior that most of users 
>>> use because we want to fix an edge case that was reported twice in the last 
>>> six years.
>>>
>>
>> I don't consider any of those *individual *conditions to trigger the 
>> problem "off the beaten path" for a bigger production Django site. All of 
>> them *combined *is obviously extremely rare, but it will effect someone 
>> else eventually*. *It doesn't sit well with me to not fix the issue. 
>> Django should be reliable in production for any combination of those 
>> conditions.
>>
>> On Wednesday, October 16, 2019 at 12:14:37 AM UTC-7, Mariusz Felisiak 
>> wrote:
>>>
>>> W dniu środa, 16 października 2019 07:53:05 UTC+2 użytkownik Harro 
>>> napisał:

 Yes, it's a complicated issue, but isn't the SQL query the ultimate 
 representation of which methods are called or not?

 Having the query evaluated during debugging has been shown to be 
 harmful in certain situations, isn't that the most important thing to fix?

>>>
>>> Current behavior is extremely valuable IMO. It is not the most important 
>>> thing to remove behavior that most of users use because we want to fix an 
>>> edge case that was reported twice in the last six years. I agree that we 
>>> can clarify this in docs. SQL query is not a solutions because not all of 
>>> ORM users know SQL. Moreover the current `repr()` shows values from DB that 
>>> we'll miss showing only SQL. You can check in the Django documentation how 
>>> users use the current `repr()` e.g. in tutorials.
>>>
>>> Best,
>>> Mariusz
>>>
>>

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/050a8686-eaa5-47e7-a9ab-d8dd6fd14844n%40googlegroups.com.


Re: Consider reverting or adding guidelines on how to use class based views for security sensitive features

2016-11-23 Thread Joachim Jablon
I'm +1 with Baptiste, Ben, Josh and João.

Also, Luke, you said :

> 1. Recognise that CBVs are much harder to reason about, and therefore 
> require much more care. 

2. Avoid using CBVs unless you really need them.
>
 
Just wanted to note that this means never. FBV vs CBV is a choice, there's 
really few cases, if any, where one or the other is *really needed*.

>From my (highly personal) point of view, some people will always be more at 
ease coding with CBV than with FBV and vice versa. FBVs are simpler at 
start, CBVs are more customizable, but in the end, you'll find a way to 
customize your FBV (a.k.a copy/pasting code) and the added complexity to 
your GCBV subclass probably won't be that high (chances are : you'll be 
adding some variable in the get_context_data(), while still calling super 
and that's it)

Maybe what we should be providing is more a way to put the complicated 
security-important logic out of a view altogether. A PasswordChanger 
class/module/... that would do the necessary work but just this work, 
independantly from how a view works. and then CBVs/FBVs would just call 
functions/methods on that. But then if there's already too much 
abstractions in CBV for the taste of some...

My 2 cents.
Joachim

Le mercredi 23 novembre 2016 06:16:24 UTC+1, Luke Plant a écrit :
>
> Hi all,
>
> First, I want to say that complex things fail in complex ways, so there 
> it's probably a fallacy to look for a single root cause. I agree with 
> various other points about mistakes that were made, but not others. 
> Particularly:
>
> On 22/11/16 12:41, Florian Apolloner wrote:
>
> Hi,
>
> On Monday, November 21, 2016 at 11:56:56 PM UTC+1, Tim Graham wrote: 
>>
>> … that the existing tests would catch this type of obviously incorrect 
>> issue.
>>
>
> I think that is the main issue here. I was also really surprised to 
> discover that the tests were missing cases like this -- then again, writing 
> good tests is hard and I know that I am personally one not to write good 
> tests usually. Either way, I think the way forward is to improve the 
> testsuite in this case. 
>
>
> I disagree with this, there was no issue with the existing test suite for 
> the FBV version. *Tests are never close to exhaustive and we should never 
> think they are or can be.* Tests are spot checks of correctness in an 
> infinite universe of possible input values. When trying to work out what 
> you should test in this vast space, there are a few sensible ways:
>
> 1. Look at the requirements, and pick spots that match each requirement. 
> In this case:
>
>- If a request has the required token, it should be allowed through 
>- If a request does not have the required token, it should not be 
>allowed through 
>
> 2. Make some guesses about well known edge cases and test them (e.g the 
> empty string case etc.). However this could be a complete waste of time
>
> 3. Look at the implementation at work out where it might go wrong. This 
> one is really important in reality. 
>
> And this is what the existing tests did. Out of the infinite number of 
> HTTP requests to test under point 1, they happened to pick a GET request, 
> which was perfectly sensible. In the case of FBVs, the control flow is so 
> clear that all of the following are equally silly tests to add:
>
>- Check that requests with query string parameter 'xyz' doesn't 
>introduce a back door
>- Check that on leap years tokens are checked 
>- Check that the security is correct for requests that are 
>specifically GET, POST, HEAD, OPTIONS, DELETE 
>
>
> The conclusion is this: in CBVs control flow is much, much less clear. In 
> the FBV the mistake would have been immediately obvious, the CBV base 
> classes obfuscated things to the point where it wasn't at all clear. To 
> write adequate tests for the CBV version, under point 3 above, you have 
> *much, 
> much more work* to do.
>
> I disagree that forcing people to write their own auth views is * 
> necessarily* worse than providing a CBV that people can customise. Given 
> this demonstration of how CBVs obfuscate control flow, it's quite possible 
> that writing an FBV from scratch will be less error prone than using a CBV 
> and subclassing, especially when we have encapsulated things like the token 
> checking. Personally I would be massively happier auditing a custom FBV 
> than a subclassed CBV, especially when you consider that the CBV subclass 
> is inheriting from a stack of classes that could change in later versions 
> of Django in some subtle way that breaks the code. Subclassing is not 
> necessarily safer, it just feels safer ("I'm only changing this one little 
> method"), and *that feeling of greater safety is itself a huge danger*.
>
> Going forward, I think we need to:
>
> 1. Recognise that CBVs are much harder to reason about, and therefore 
> require much more care.
> 2. Avoid using CBVs unless you really need them.
> 3. Recognise that tests for FBVs are 

Re: New deferred loading implementation (Was: [Review Request] Added support for database delegated fields (like AutoField))

2016-11-08 Thread Joachim Jablon
We were having a discussion on this matter on the Django under the Hood 
Slack channel, and there's a design decision that I think I can't take by 
myself.

There are 2 related subject : the "readonly" part and the "auto_refresh" 
part. Readonly means that the database won't try to write, autorefresh 
means that when creating /updating an instance, the value will be fetched 
from the database automatically

There was a debate on whether the readonly behaviour should imply 
auto_refresh, and there are cases that are good candidates for auto_refresh 
without readonly (autofields, serial fields etc.).

What I'd suggest, if we can get an agreement on this, would be to define 
those 2 behaviours completely independently (possibly as 2 different PRs, 
each with their tests and docs and such).

Auto_refresh could then be used for autofields with the quirk that some SQL 
DBs, will provide last_insert_rowid() (sqlite) or LAST_INSERT_ID() (mysql), 
while other use RETURNING (PGSQL / Oracle) allowing to use a single query.

Readonly, on its side, would only be a simple independent feature and its 
behaviour would do what the title says, no more, no less.

Do you think I need to do a DEP on this ?
If not, do you think it's ok if https://github.com/django/django/pull/7515 is 
changed to only do the readonly part, and we make another PR (and possibly 
ticket) for the "auto_refresh" part ?

While it would be nice to have both landing at the same time on master, I 
feel there's nothing blocking if they land at separate time, even in 
separate versions if need be (the freeze on Django 1.11 is coming fast, 
IIRC).

Are there usecases that I'm missing and that could not be expressed as a 
mix of readonly and auto_refresh ?

Le lundi 7 novembre 2016 15:35:38 UTC+1, Aymeric Augustin a écrit :
>
> On 7 Nov 2016, at 13:44, Joachim Jablon <ewjo...@gmail.com > 
> wrote: 
>
> > My own personal opinion: 1. refesh by default, add an argument to 
> "model_instance.save()" to opt-out. 2. readonly 
>
> I agree. 
>
> -- 
> Aymeric. 
>
>

-- 
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/3db52e5b-54db-4736-845b-58a22b6f015c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: New deferred loading implementation (Was: [Review Request] Added support for database delegated fields (like AutoField))

2016-11-07 Thread Joachim Jablon
So the PR followed here : https://github.com/django/django/pull/7515

The PR is based on the idea defined by Ben just above.

It raised 2 questions which I'd like to bring to your attention :

1. When inserting or updating a model instance that contains one or more 
readonly fields, should we fetch the readonly fields values from the 
database (instance.refresh_from_db(fields=readonly_fields))
Pro : 
- Consistent with AutoFields
- Easier to work with
- Can be done without an extra query for PGSQL using RETURNING (but it's 
out of scope for this ticket. That being said, it could be done within the 
1.11 scope)
Con :
- Intruduces an extra select query for models that have at least 1 readonly 
field.
- Can be easily done manually (especially if we provide a model instance 
such as ".refresh_readonly_fields()")
Alternative choice :
Give the choice to the user (in this case we still need to choose the 
default behaviour)

It's worth noting that the only affected models are those that include a 
"readonly" field, so that's not an extra select for everyone but only for 
the adopters of the new feature. It's also worth noting that the select is 
affecting a single element, using its pk, and targetting only the readonly 
fields.

2. For now this feature is named "readonly" and its default value is False. 
There was a suggestion to rather user the word "writable" and have its 
default value to True. I think both terms carry the same information.
Pro "readonly"
It's easier to name things in the code ("readonly fields" is a simpler 
concept than "non-writable fields" or "unwritable fields")
Pro "writable"
I'm not sure, but there were 3 :+1: on the GitHub comment so it's probably 
clearer

My own personal opinion: 1. refesh by default, add an argument to 
"model_instance.save()" to opt-out. 2. readonly

Le samedi 5 novembre 2016 16:17:31 UTC+1, Ben Cole a écrit :
>
> As discussed with many core team members (Simon Charette, Josh Smeaton, 
> Marc Tamlyn, Tim Graham) at DUTH 2016 sprints, myself and Joachim 
> Jablon have proposed a new, simpler solution to this. See 
> https://code.djangoproject.com/ticket/27446
>
> The proposal therefore is to add a `readonly` option to the base `Field` 
>> class that when `True` would strip these fields from being compiled to SQL 
>> during `INSERT`s and `UPDATE`s. This allows for a very simple change that 
>> covers all possible write queries that Django may perform (including 
>> bulk_*).
>> There exists a proof of concept 
>> https://github.com/novafloss/django-readonly-field
>
>
> PR to follow soon...
>
> On Friday, 12 February 2016 12:32:11 UTC, Anssi Kääriäinen wrote:
>>
>> On Thursday, February 11, 2016 at 2:41:44 PM UTC+2, Florian Apolloner 
>> wrote:
>>>
>>> Oh, I somewhat missread and though there would be a new DEFERRED 
>>> argument, the backwards issue is easy enough though:
>>>
>>>  * Unless I miss something, YourModel.__init__ is Model.__init__ if the 
>>> user didn't change it -> pass is DEFERRED
>>>  * If the user changed it check for model._meta.new_style_deferred and 
>>> continue accordingly
>>>  * Raise a warning if the __init__ is a custom one and 
>>> new_style_deffered is not set…
>>>
>>
>> If we are going to introduce a backwards compat system for this, then I 
>> think I want to get rid of calling Model.__init__ at all when loading from 
>> DB. We get faster model initialization, and conceptually loading from DB is 
>> like unpickling which (correctly) doesn't call __init__.
>>
>> However, based on the comments in the PR, I think we are going to just 
>> document the change to __init__ and skip deprecation.
>>
>>  - Anssi
>>  
>>
>>>
>>> On Thursday, February 11, 2016 at 1:38:44 PM UTC+1, Florian Apolloner 
>>> wrote:
>>>>
>>>>
>>>>
>>>> On Thursday, February 11, 2016 at 10:51:59 AM UTC+1, Anssi Kääriäinen 
>>>> wrote:
>>>>>
>>>>> Before doing any further work on this we should decide if the 
>>>>> Model.__init__() problem is bad enough to stop this cleanup, and if so, 
>>>>> do 
>>>>> anybody have any ideas of how to avoid the problem?
>>>>>
>>>>
>>>> I do not think Model.__init__() is anywhere near public API, add it to 
>>>> the release notes and be done with it, worst case add a try/except around 
>>>> it…
>>>>
>>>> Cheers,
>>>> Florian 
>>>>
>>>

-- 
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/2e8733b3-8336-4dad-909c-6a97711a9aa3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Docs and search : Split the results by category

2016-01-04 Thread Joachim Jablon
Hello,

(not sure the question should be asked there or as a ticket of 
djangoproject.com)

Seing this 

 answer 
by Daniele Procida explaining how docs are organized made me realize that 
often, when searching for something in the docs, I know if I'm searching 
for an how-to doc, a reference doc or a topic doc. I'd find helpful to have 
the search results splitted into several categories, so that I can ignore 
(at first) the information that will be too low or high level for what I'm 
looking for. Am I the only one thinking this ? If not, I can try making it.

What I'd have in mind : the 2 or 3 first results of each category plus a 
link to have more results for each category, and a link to have all results 
mixed, as it works now.

(I'm not sure but I believe that at some point a while ago, I searched for 
"input" and the HowTo for "Form" was somewhere about the 10ish result. I'd 
like to check with the docs search but it seems the search is down for now)

Yours,

ewjoachim

-- 
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/d1b550f1-55b5-4a43-971f-636d39510a24%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Add an optional parameter to values() that returns a nested dictionary for foreign keys

2015-11-25 Thread Joachim Jablon
Marten's suggestion is quite interesting for providing a way to tell which 
data you want nested and which data you don't. Plus, this form might be 
interesting to solve another problem : how would Django know if we want :

{"id": 1
 "first_name": "first name",
 "last_name": "last name",
 "hometown": {
  "id": 1,
  "name": "town name",
  "country": 3
  }
}


# or


{"id": 1
 "first_name": "first name",
 "last_name": "last name",
 "hometown": {
  "id": 1,
  "name": "town name",
  "country": {
"id": 3,
"name": "country name"
  }
  }
}



Limiting the nesting to a single level would be an arbitrary decision and 
users should be able to control this (IMHO)

So we could have a "level" argument that would say how many levels deep it 
will search but then what if you want SOME nesting in some branches, not in 
others, like : 

{"id": 1
 "first_name": "first name",
 "last_name": "last name",
 "hometown": {
  "id": 1,
  "name": "town name",
  "country": {
"id": 3,
"name": "country name"
  }
  },
  "father": 4
}


(here, "father" is another FK that we don't want expanded ?

Maybe a syntax like :

N("person", "person__hometown", "person__hometown__country")
Note : this might not be equivalent to N("person__hometown__country"), that 
you could use if you want ONLY the nested "country"

I'd like that.

And it's compatible with the suggestion of using **kwargs for aliasing (for 
the top level element of the dict, at least)

Le mercredi 25 novembre 2015 17:53:25 UTC+1, Marten Kenbeek a écrit :
>
> I think it'd be more consistent with other parts of the ORM to use 
> **kwargs to specify aliases. For nested data you can use an object, say N, 
> similar to Q and F objects:
>
> Articles.objects.filter(id=1).values('body', N('author'), my_custom_title=
> 'title')
>
> I'm no ORM expert, but could something like this be possible by allowing 
> expressions in values() and using custom output fields?
>
> On Wednesday, November 25, 2015 at 5:09:29 PM UTC+1, Moenad wrote:
>>
>> Well, switch the field name aliasing to a dictionary without hijacking 
>> **kwargs ?
>>
>> I prefer the following:
>>
>> Articles.objects.get(id=1).values(’title’, ’author’, ‘body', 
>> alias={"title": "my_custom_title"}, nested=True)
>>
>>
>> On Wednesday, November 25, 2015 at 5:43:51 PM UTC+2, Tim Graham wrote:
>>>
>>> There's an accepted ticket for adding aliasing to values(): 
>>> https://code.djangoproject.com/ticket/16735
>>>
>>> The current patch there hijacks values() **kwargs for the mapping of 
>>> renamed fields which would prevent adding other kwargs like "nested" 
>>> without disallowing those values as aliases. I guess we may want to rethink 
>>> that approach.
>>>
>>> On Wednesday, November 25, 2015 at 10:20:37 AM UTC-5, Bobby Mozumder 
>>> wrote:

 I could also use a couple of enhancement to this:

 1) Allow renaming of keys, instead of using the database column names.  
 2) Allow callbacks functions (or lambdas) to convert output values to 
 another format if needed.

 With this, I could send the queries results right to JSON outputs.

 -bobby

 On Nov 25, 2015, at 9:05 AM, Moenad  wrote:

 Currently, after calling values() and the query executes, the output is 
 a single level dictionary, including foreign keys. I propose adding an 
 extra parameter for values, or at least values_list, where if it's set to 
 true, a nested dictionary will be returned when there's a foreign key.

 Example:

 Person model with the following fields: first_name, last_name and 
 hometown (foreign key)
 Hometown model with the following fields: name

 A single record from Person.objects.values() will looks like this

 {"id": 1
  "first_name": "first name",
  "last_name": "last name",
  "hometown__id": 1,
  "hometown__name": "town name",
 }


 I propose adding a nested optional parameter to values, where a single 
 record from Person.objects.values(nested=True) will look like

 {"id": 1
  "first_name": "first name",
  "last_name": "last name",
  "hometown": {
   "id": 1,
   "name": "town name"
   }
 }


 This feature is needed given that most APIs these days are nested, 
 while it's simple to implement, I think it's much better to have it a 
 built-in django feature.


 -- 
 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 http://groups.google.com/group/django-developers.
 To view this discussion on the web visit 
 

Re: Feature: URL template tag, optional parameters

2015-06-18 Thread Joachim Jablon
But what if we need to distinguish between "" and None ?

E.g. :

url(r'^blog/page/(?P\d+)/$', views.page, name='view-page'),
url(r'^blog/page/(?P\d+)/subpage(?P.*)/$', views.page, name
='view-page'),

So that makes :
reverse("view-page", page_pk=12) == "blog/page/12/"
reverse("view-page", kwargs={page_pk:12, option:""}) == "
blog/page/12/subpage/"

How would you distinguish between those 2 in the template ?






*Joachim JablonIngénieur Systèmes+33 (0)6 75 66 76 84+33 (0)1 84 17 31
20SMART IMPULSE48 rue René Clair75018 PARISwww.smart-impulse.com
*

2015-06-11 21:07 GMT+02:00 Tim Graham :

> I think you'll have to try implementing it to see if it's feasible.
>
>
> On Thursday, June 4, 2015 at 5:44:54 AM UTC-4, erez@gmail.com wrote:
>>
>> Hi,
>>
>> AFAIK, the recommended way in Django to handle multiple urls with the
>> same view, is to have optional parameters. (
>> https://docs.djangoproject.com/en/1.8/topics/http/urls/#specifying-defaults-for-view-arguments
>> )
>>
>> For example,
>>
>> in url.py:
>>
>> url(r'^blog/page/(?P\d+)/$', views.page, name='view-page'),
>> url(r'^blog/page/(?P\d+)/(?P[a-zA-Z0-9_.-])/$', views
>> .page, name='view-page'),
>>
>> in views.py:
>> # View (in blog/views.py)
>> def page(request, page_pk, page_slug=""):
>>
>>
>> However, in the templates, the url template tag can't accept optional
>> parameters.
>> So instead of writing {% url 'view-page' page_pk page_slug %}, you need
>> to write {% if page_slug=="" %}{% url 'view-page' page_pk %}{% else %}{%
>> url 'view-page' page_pk page_slug %}{% endif %}
>> On the project I'm working on I have 2 or more optional parameters, and
>> it's a real nuisance.
>>
>> The url tag could have done this logic by itself. If any of its inputs
>> are blank or none, disregard it.
>>
>> Same question from Stack Overflow (not asked by me) :
>> http://stackoverflow.com/questions/16870884/url-template-optional-param
>>
>> Thanks
>>
>  --
> 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 http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/9c67b646-ee16-4f82-a545-9e4178d4e7dc%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 http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAGuotBH93NGMvviQwJ%2BZDU8pCPb8Vr63UM7yY2fGpOKTQzHtUw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Fate of sql* management commands

2015-03-31 Thread Joachim Jablon
Not sure about this but wouldn't "RunPython" actually keep you from getting 
an proven accurate result ? Even if we could imagine running the migration 
in a transaction and capturing the SQL that would be run, it would be 
dependent on the current data in the DB among other thing (could even be 
dependant on the meteo, or random variables or anything)...

And given that you can (as the doc says) use the scheme editor inside a 
RunPython migration, who knows what the proper SQL would be ?

(again, I may be missing a point there)

-- 
Joachim Jablon


Le lundi 30 mars 2015 06:00:10 UTC+2, Andrew Godwin a écrit :
>
> I must also point out that the sqlmigrate command will still get you SQL 
> from migrations, though it sadly lacks the ability to take a range of 
> migrations, optimise them down, and output the SQL for _that_ - that's 
> probably the best thing for us to aim towards, and will almost entirely 
> recreate the functionality of sqlall without actually being wrong (sqlall 
> doesn't understand things like data migrations or custom SQL sections of 
> migrations, and so will sadly be wrong in if you have anything like a 
> moderately complex migration set).
>
> The plus side is that it would be possible to do this sort of thing as a 
> third-party command/backport if we wanted to have it usable on 1.8 and even 
> 1.7 (the 1.8 feature set is frozen now, or I'd suggest quickly squeezing it 
> in).
>
> Andrew
>
> On Sun, Mar 29, 2015 at 4:57 PM, Russell Keith-Magee <
> rus...@keith-magee.com > wrote:
>
>>
>> On Mon, Mar 30, 2015 at 1:36 AM, Pkl <chambon...@gmail.com > 
>> wrote:
>>
>>> Hello all,
>>>
>>> I've noticed that all sql* commands that were in django core management 
>>> have disappeared from the latest sources.
>>>
>>> However, even though Django now has builtin south-style migrations now, 
>>> these *sql** commands (especially sql-all) were priceless to debug 
>>> differences between the models and the actual DB schema. 
>>> Especially as long as some django apps haven't caught up with all these 
>>> recent changes - I myself ended up with missing tables after scrupulously 
>>> following upgrade instructions, with "manage.py migrate" saying "all is 
>>> fine".
>>>
>>> *What is the new way to dump the sql schema of currently installed 
>>> django appz ?* It'd maybe be worth that I provide a doc patch to inform 
>>> users about it.
>>> *If there is none, is there an agreement to restore at least sqlall and 
>>> sqlclear ?*
>>>
>>  
>> No, there isn't :-)
>>
>> Also what was the reasoning behind, on the first, place, blocking access 
>>> to these utilities in django1.7 (they seemed to work, when I forced them), 
>>> and now removing them ? A simple warning about the new "migrations" stuffs 
>>> (or a "--force" argument) would most probably have sufficed to prevent 
>>> improper usage of these sql* commands, while letting django users getting 
>>> the info they need to move forward, wouldn't it ?
>>>
>>  
>> Right now, there are two (almost) completely independent implementations 
>> of SQL table generation logic: 
>>
>>  * The legacy version, used by syncdb, sqlall, et al
>>
>>  * The new version, used by migrate.
>>
>> In the past, there was a good reason for sqlall to exist - you needed to 
>> be able to run syncdb and create tables. The only time you ever called 
>> "CREATE TABLE" was when a table was fresh, and after that, you never issued 
>> any command related to the table schema. It didn't hurt anyone to allow 
>> sqlall to generate "what would the CREATE TABLE be if we had clean 
>> databases", and it was at least partially useful as a migration assistant, 
>> so the sqlall functionality was available.
>>
>> In the era of migrations, there's still a need to do a CREATE TABLE, but 
>> the table creation logic is a lot more complex, because it's not *just* a 
>> CREATE TABLE that is needed; all the ALTER TABLE statements are needed to 
>> add/remove columns as well. 
>>
>> It would probably be *possible* to refactor manage.py sqlall to use the 
>> new table creation logic; but the question would then be "why do you want 
>> it"? manage.py migration *should* be handling all your table creation and 
>> migration functionality, so the need to manually diff a schema shouldn't 
>> exist. If you've hit problems with "manage.py migrate" *not* identifying 
>> migrations, then that's a much bigger pro