Re: App-loading: Pragmatic concerns about default AppConfig objects and ready() implementations

[Russell Keith-Magee]

For tracking purposes, I've opened a ticket:

https://code.djangoproject.com/ticket/21829

Russ %-)

On Sun, Jan 19, 2014 at 10:08 PM, Aymeric Augustin <
aymeric.augus...@polytechnique.org> wrote:

> On 19 janv. 2014, at 13:46, Russell Keith-Magee 
> wrote:
>
> On Sun, Jan 19, 2014 at 5:48 PM, Aymeric Augustin <
> aymeric.augus...@polytechnique.org> wrote:
>
>
> We can certainly use AppConfig.ready() for new features such as the checks
>> framework. It’s reasonable to ask our users to edit their configuration to
>> take advantage of a new feature. If they don’t, they don’t lose anything
>> compared to the previous version of Django. Backwards-compatibility is
>> preserved.
>>
>
> I'm afraid I don't see *how* we can use ready() for new features. Checks
> are *definitely* off the table for using the feature.
>
>
> Oops, I missed that the checks framework replaces *existing* validation
> features. Sorry.
>
> Also there might be good reasons for not using an AppConfig.
>>
>
> I'd be interested in hearing what these are.
>
>
> For instance, if you’re using a custom AdminSite, you may want the admin’s
> auto-discovery not to kick in. Honestly, it’s a theoretical argument, I’m
> not sure it really matters in practice.
>
> 2) Use a naming convention - require that the default app configuration
>> must be called AppConfig, so we can always try to load
>> myapp.apps.AppConfig. If this object doesn't exist, install the system
>> default AppConfig (as it does currently).
>>
>>
>> Jannis vetoes that idea and I believe he’s right. Auto-importing modules
>> for discovery is a bad idea. “One does not simply import modules.” ;-)
>>
>
> I'll certainly appreciate (and largely agree with) that logic. My concern
> is the pragmatic one of not being able to use ready() as a clean approach
> to a problem I'm currently solving in a less than ideal way.
>
>
> In fact, that was part of the original implementation. My main motivation
> was to make the transition easier for pluggable apps, and especially
> contrib apps. That’s also what you’re looking for right now.
>
>  3) Do 2, but with a level of indirection. Let the user call the AppConfig
>> module whatever they want, but allow the apps module to define a
>> 'default_app_config' attribute that provides the string to use as a default
>> AppConfig.
>>
>>
> we could put default_app_config in the application’s root package but
>> that’s ugly.
>>
>
> I agree that it's less than ideal, but I don't think a string definition
> in __init__is *more* ugly than implicit imports and registrations happening
> in the __init__  - in fact, I'd say it's a whole lot *less* ugly, because
> it's just identifying a string in a module that was already being imported,
> and using that as a trigger to import another specific module (one that the
> end user could reference explicitly if they were so inclined).
>
>
> Indeed, this convention provides a way for pluggable apps to select a
> default AppConfig class without requiring end-users to change
> INSTALLED_APPS.
>
> It would make the transition much easier. For instance, at the moment,
> there’s no way to configure the debug toolbar in a way that works both on
> 1.6 and 1.7.
>
> I’m at least +0, and probably +1 on the idea, but I’d like to hear the
> opinion of other people who contributed to the current design, especially
> Jannis.
>
> --
> Aymeric.
>
>  --
> You received this message because you are subscribed to the Google Groups
> "Django developers" 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/45F925BB-699F-4D7D-9871-ACD766E85926%40polytechnique.org
> .
>
> For more options, visit https://groups.google.com/groups/opt_out.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/CAJxq849VODsC8-gXRcRxBpa1aBy587J1a%3Deh6hTBLE3R-p-kbQ%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Django Security & OWASP Project

[Jacob Kaplan-Moss]

Hey Michael --

This sounds right up my ally. I'll jump on the list and post some more info
over there.

Jacob

On Tuesday, January 14, 2014, Michael Coates 
wrote:

> Django Developers,
>
> Hello! Over at OWASP I've started a framework security project. Our goal
> is to capture the security posture, options and capabilities of different
> frameworks. Through this we can educate developers on how to enable
> security controls in the framework and also work with frameworks to gain
> adoption of any missing capabilities.
>
> When I was leading the security team at Mozilla we worked with Django a
> ton. You guys have always been on the leading edge of framework security.
> It was an easy choice to start with Django for the OWASP framework security
> project.
>
> Would any of you be interested in helping out with our project?
>
> Ways to help:
> 1. Information gathering - we're putting together a standard list of
> security controls in frameworks and django's support (we'll move to other
> frameworks with this model). You can provide info here:
> https://docs.google.com/a/owasp.org/spreadsheet/ccc?key=
> 0AhSfMVkfLvsldEltRUEwMkUydVVrMkNyVW1vbGxLaXc#gid=0
>
> - Please suggest additional security controls we should have in column A
> - We also need to capture if Django supports the different controls, the
> version added, default options, etc
>
> 2. Assistance with adding missing controls
> This will come later, but if we find any missing controls it would be
> great to understand the best way to work together to get them added.
>
> 3. Join the mailing list
> https://lists.owasp.org/mailman/listinfo/owasp_framework_security_project
>
> Any thoughts or ideas are welcomed. We're in the beginning and will
> continue to flush out the project as we go.
>
>
>
> Thanks!
>
> --
> Michael Coates
> @_mwc
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com 'cvml',
> 'django-developers%2bunsubscr...@googlegroups.com');>.
> To post to this group, send email to 
> django-developers@googlegroups.com '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/d5305915-8298-465a-bc8a-ec2df73b3587%40googlegroups.com
> .
> For more options, visit https://groups.google.com/groups/opt_out.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/CAK8PqJHeKNahO2qrfnUE4drog6N%2BbAOdGQj%3D6tZzobsiQQQh%2BA%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Merging GSoC 2013 - Validation Refactor

[Russell Keith-Magee]

Hi all,

For notification purposes - this has been committed as of d818e0c, ready
for 1.7 alpha.

https://github.com/django/django/commit/d818e0c9b2b88276cc499974f9eee893170bf0a8

The final commit integrated a bunch of suggestions from a couple of
reviewers, some Python 3 fixes, and a slight rework of the Django 1.6
TEST_RUNNER compatibility fix (to reduce the number of false positives that
check was providing).

On the TODO list before 1.7 final:

 * There's some ongoing discussion about the best way to integrate the
check registration process with the new app-loading ready() method;

 * The text of the actual messages raised by the checks framework needs a
final audit.

 * There's lots of room for improvement in documentation, including an
index of checks that Django will raise

Tim has also suggested some possible improvements to the process for
silencing check messages.

Thanks again to Christopher Medrela for his excellent work in GSoC 2013,
without whom none of this would have been possible.

Yours,
Russ Magee %-)



On Sun, Jan 19, 2014 at 12:50 AM, Russell Keith-Magee <
russ...@keith-magee.com> wrote:

> Hi all,
>
> After a long delay, I'm finally ready to merge the result of Christopher
> Medrela's 2013 Summer Of Code Project - a refactor of Django's validate
> management command.
>
> A huge thanks to Christopher for his excellent work over the summer - he
> made my job as mentor very easy.
>
> For those that didn't follow along with the project itself - Django's
> validate management command was a single monolithic block of code that
> checked for name collisions, certain allowable attribute values, and so on.
> It wasn't extensible in any way.
>
> The purpose of this refactor was to break this monolithic method up into
> smaller parts, and along the way, provide a way for other apps/libraries to
> hook in and provide their own checks. So, for example, GenericForeignKeys
> now get validation support - this wasn't previously possible because they
> are part of a contrib app, and therefore couldn't be referenced in the core
> codebase.
>
> So - if you've got a field, model base class or model manager that needs
> custom validation that you'd like to be reported in the same way as
> ./manage.py validate has historically reported, you now have a way to do so.
>
> Or, if you've got some other system check that you'd like to perform (for
> example, performing a security audit), you can hook in to the same tools.
>
> As part of this refactor, we've also dealt with the manifold overloading
> of the term "validate" in Django's codebase. We've deprecated "validate" in
> preference for the "check" command.
>
> A side effect of this is that the version update checks introduced in 1.6
> are now guaranteed to run every time you do runserver. To make sure you
> aren't needlessly bothered once you've dealt with these issues, individual
> message types can be silenced. So, for example, when you do a version
> update, you may be warned about a change; once you're satisfied that you've
> met that requirement, you can silence that error and never see it again.
>
> We also allow for different severity levels: DEBUG, INFO, WARNING, ERROR,
> CRITICAL. This provides a lot more options for reporting problems that
> aren't show-stoppers, but probably warrant attention (e.g., easily
> identifiable performance issues).
>
> I've created a clean PR based on a rebased version of the code that has
> been brought up to date with trunk:
>
> https://github.com/django/django/pull/2181
>
> There are a still a couple of minor issues that may need to be addressed -
> in particular, I want to do a cleanup of the language for the messages
> themselves, and the documentation would probably benefit from another pair
> of eyes. However, I'm happy with the way the core has shaken out; I want to
> get it into core so it gets more exposure.
>
> Tim's review a couple of days ago also pointed out a couple of interesting
> features (in particular, adding a command-line flag to silence specific
> warnings) which might be nice to have, but shouldn't affect core
> functionality.
>
> Barring objections, I'm planning to land this in time for the 1.7 alpha
> (so Andrew - no cutting releases until I've had a chance to push the merge
> button :-)
>
> So - final chance for comments and feedback before this lands!
>
> Yours,
> Russ Magee %-)
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/CAJxq848f1o7AFiVju44mtR6A8EVg45Bk0gqG9znC-GqfXnn3XQ%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Testing parameters in database settings

[Shai Berger]

On Wednesday 15 January 2014 00:02:52 Michael Manfre wrote:
> On Tue, Jan 14, 2014 at 3:26 PM, Shai Berger  wrote:
> >
> > Your suggestion, if I understand it correctly, gives the user two options:
> > 
> > 1) Use separate credentials, and perhaps even a separate service, for
> > testing;
> > this implies that the test database stays alive all the time.
> 
> As it stands, the privileges for running the tests are greater than those
> needed to run syncdb (or runserver), so I'm not sure what point you are
> trying to make.

I did not understand your proposal correctly; I had thought you intended to 
put the credentials for accessing the test database on equal footing with the 
credentials for accessing the main database. 

> This assumes you're not using Oracle and need to provide the
> slew of other configuration values, which should really be moved to
> OPTIONS.
> 

I am still undecided about this one. We have backend-agnostic test-only 
parameters (MIRROR and DEPENDENCIES, at least), we have backend-specific 
parameters which apply to both testing and "production" (which usually go into 
OPTIONS). I want the test-only parameters tucked away, and I'm not sure in 
which of the above the test-only backend-specific parameters should go; but my 
tendency is toward 'TEST'.

> > 2) Write configurations with repetitions or some other awkwardness, e.g.
> >
> > [example redacted]
> > 
> 
> This is not what I envisioned. By separate aliases, I don't mean to just
> add more test aliases in with your existing settings. What I mean is that
> each database alias defined in a settings file should define a single
> connection configuration, not potentially two different configurations. If
> you need a different configuration for testing, put them in a separate
> configuration file.
> 
>  Example:
> 
> # settings.py - for local development
> DATABASES = {
>   'default': {
> ...
># 'CAN_USE_FOR_TESTS': False, # Protect database from unit test trampling
>   }
> }
> 
> 
> # test_settings.py - for unit tests
> DATABASES = {
>   'default': {
> 'ENGINE': 'oracle',
> 'NAME': 'test_my_db', # This was TEST_NAME
> 'USER': 'dba_user', # This is a user with credentials to drop a database
> ...
> 'OPTIONS': {
>   # backend specific settings belong here
>'TEST_CREATE': True,
>'TEST_TBLSPACE': 'test_',
>'CHARSET': 'utf-8', # Or leave named as TEST_CHARSET
>...
> },
> 'CAN_USE_FOR_TESTS': True, # DB is safe to trample with tests
>   }
> }
> 

So, If I understand correctly (this time), your "test settings" are a strict 
superset of your "local development" settings.

But I don't like the separation at all. It makes sense for some enterprise 
environment, but not for a setting where each developer has their own database 
installation. Making it in any concrete way will require the project to make 
some decisions about the structure of the settings files, which we have avoided 
for a very long time -- it is certainly out of scope of a little settings 
clean-up. And some projects will just want to do something like 12factor, 
where all credentials come from the environment rather than settings files, and 
this is how you get different enterprisey settings.

If your suggestion is to make the separate files concretely supported by Django 
-- e.g. use a different settings file, by default, for the test commands -- 
please flesh it out. I suspect I still don't quite follow where you're going 
with this.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/201401200012.02517.shai%40platonix.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Ticket #21751 review requested

[Shai Berger]

On Sunday 19 January 2014 10:44:32 Michael Manfre wrote:
> On Sun, Jan 19, 2014 at 5:23 AM, Shai Berger  wrote:
> > Still, spreading with-blocks all over the code for this looks very ugly to
> > me.
> > I'd rather add an execute_single() or execute_and_close() method to the
> > cursors instead. Perhaps even only as private API, as that usage is mostly
> > common within Django's code.
> 
> You would rather deviate from PEP 249 to avoid requiring the users of a
> cursor to explicitly close it? 

No. I'd rather extend the interface defined by PEP 249 in order to reduce the 
amount of RY in the code. Your PR has at least 10 instances of 

with connection.cursor() as cursor:
cursor.execute(...)
# block ends here

and I think around 10 more of variations on the theme

with connection.cursor() as cursor:
cursor.execute(...)
do_something_with(cursor.fetchone())

In similar, though less general fashion, I would add to 
tests/introspection/tests.py a simple module-level function:

def get_table_description(*args):
with connection.cursor as cursor:
return connection.introspection.get_table_description(cursor, 
*args)

There are many other instances in the PR where a cursor defined at the with 
statement is used for more than one database command; I see those as net 
improvement.

(I must admit that my initial reaction was "colored" by your other PR[1], 
where these one-line-with-blocks seem to be a larger percentage of the with-
blocks. Still).

Shai.

[1] https://github.com/django/django/pull/2143

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/2404579.hcV3zBsRXv%40deblack.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Ticket #21751 review requested

[Michael Manfre]

On Sun, Jan 19, 2014 at 5:23 AM, Shai Berger  wrote:
>
> Still, spreading with-blocks all over the code for this looks very ugly to
> me.
> I'd rather add an execute_single() or execute_and_close() method to the
> cursors instead. Perhaps even only as private API, as that usage is mostly
> common within Django's code.
>

You would rather deviate from PEP 249 to avoid requiring the users of a
cursor to explicitly close it? Interesting...

Regards,
Michael Manfre

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/CAGdCwBtzXMjwGnAAC33pKMRANWS%3D%2BGtfRbjcLWMj8Wgssq1SBw%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Improving aggregate support (#14030)

[Michael Manfre]

There are a few other things that I need to try and get in before the 1.7
feature freeze. Since this is not slated for 1.7, I'll take a look a the PR
after the freeze and see if the comments I had still apply.

Regards,
Michael Manfre


On Sun, Jan 19, 2014 at 8:00 AM, Josh Smeaton wrote:

> I've finally sent a PR, so if you're still able, I'd love to see the
> specific comments you had.
>
> https://github.com/django/django/pull/2184
>
> Cheers,
>
> - Josh
>
> On Sunday, 12 January 2014 10:35:58 UTC+11, Michael Manfre wrote:
>
>> With minor tweaks to django-mssql's SQLCompiler, I am able to pass the
>> aggregation and aggregation_regress tests. If you create the pull request,
>> comments can be attached to specific lines of code.
>>
>> I haven't had time to do a full review of your changes, but my initial
>> questions and comments are:
>>
>> - Why were the aggregate specific fields, properties, etc. added to
>> ExpressionNode, instead of being on django.db.models.aggregates?
>> - Why does an ExpressionNode containing any Aggregates need to pollute
>> the entire tree with that knowledge?
>> - Instead of using is_ordinal and is_computed (confusing name, why not
>> is_float), you could eliminate those and define output_type for each
>> Aggregate that does something special.
>> - What does is_summary mean?
>>
>> Regards,
>> Michael Manfre
>>
>>
>> On Sat, Jan 11, 2014 at 3:06 AM, Josh Smeaton wrote:
>>
>>> A milestone has been reached. The diff is here: https://github.com/
>>> jarshwah/django/compare/akaariai:lookups_3...aggregates-14030?expand=1
>>>
>>>- Existing annotations and aggregations working as they were
>>>- Complex annotations and complex aggregations now work
>>>- Complex annotations can now be referenced by an aggregation
>>>- Cleaned up the logic relating to aggregating over an annotation
>>>
>>> See the tests for the new functionality here: https://github.com/
>>> jarshwah/django/blob/aggregates-14030/tests/aggregation/tests.py#L662
>>>
>>> Things I still need to tackle:
>>>
>>>- GIS
>>>- Custom backend implementation of aggregates and annotations
>>>- Annotations that aren't aggregates. I think this should be
>>>extremely easy, but I might not be aware of some underlying difficulties.
>>>
>>> Things that should be done in the future, as I don't think there will be
>>> time for this just now:
>>>
>>>- Change order_by (and other functions) to accept Expressions.
>>>
>>> I would think that, as it stands, this code is ready for review with the
>>> above caveats. If there are any backend authors reading (mssql,
>>> django-nonrel), I would encourage you to look at the implementation here,
>>> and let me know if there's anything glaringly obvious (to you) that is
>>> missing or broken.
>>>
>>> Regards,
>>>
>>> - Josh
>>>
>>> On Thursday, 9 January 2014 00:33:08 UTC+11, Josh Smeaton wrote:

 I'll preface by saying it's late for me, and I'm about to sign off, so
 any mistakes below I will correct tomorrow after some sleep.


> To write a custom aggregate one needed two classes, one for user
> facing API, one for implementing that API for SQL queries.


 I've taken note of this already, and that is the reason I've kept the
 sql.Aggregate class where it is, though it's not used internally anymore.
 There are two options that I can see right now:

 - The non-compatible way involves moving the sql_function attribute to
 the Aggregate from the SQLAggregate, and everything should work fine
 - The backward-compatible way might involve re-adding the
 "add_to_query" function, and using the SQLAggregate to monkeypatch the
 Aggregate by copying over the sql_function attribute. I would have to
 experiment and test this though. I assume this would be the more
 appropriate option. We can include a DeprecationWarning to phase it out.


> Another case you should check is how GIS works *without* rewriting
> anything in django.contrib.gis


 Good idea, that's how I'll approach the aggregate stuff. Not sure that
 I can leave it completely alone due to Expressions though, but we'll see
 (and hope).

 I'll try to check your work in detail as soon as possible.
> Unfortunately I am very busy at the moment, so I can't guarantee anything.


 Totally understandable, I appreciate your comments regardless. It's
 just unfortunate there aren't more people deeply familiar with the ORM -
 it's bloody complicated :P

 I'm now to the point where all tests in aggregate, aggregate_regress,
 expressions, expressions_regress, and lookup pass. I've got a small problem
 with .count() regressions failing, but am working through that now.
 Tomorrow I should be able to start playing with new functionality and
 writing new tests. I'll also take a look into running tests on GIS and
 fixing any 

Re: App-loading: Pragmatic concerns about default AppConfig objects and ready() implementations

[Aymeric Augustin]

On 19 janv. 2014, at 13:46, Russell Keith-Magee  wrote:

> On Sun, Jan 19, 2014 at 5:48 PM, Aymeric Augustin 
>  wrote:
> 
> We can certainly use AppConfig.ready() for new features such as the checks 
> framework. It’s reasonable to ask our users to edit their configuration to 
> take advantage of a new feature. If they don’t, they don’t lose anything 
> compared to the previous version of Django. Backwards-compatibility is 
> preserved.
> 
> I'm afraid I don't see *how* we can use ready() for new features. Checks are 
> *definitely* off the table for using the feature.

Oops, I missed that the checks framework replaces *existing* validation 
features. Sorry.

> Also there might be good reasons for not using an AppConfig.
>  
> I'd be interested in hearing what these are.

For instance, if you’re using a custom AdminSite, you may want the admin’s 
auto-discovery not to kick in. Honestly, it’s a theoretical argument, I’m not 
sure it really matters in practice.

>> 2) Use a naming convention - require that the default app configuration must 
>> be called AppConfig, so we can always try to load myapp.apps.AppConfig. If 
>> this object doesn't exist, install the system default AppConfig (as it does 
>> currently). 
> 
> Jannis vetoes that idea and I believe he’s right. Auto-importing modules for 
> discovery is a bad idea. “One does not simply import modules.” ;-)
> 
> I'll certainly appreciate (and largely agree with) that logic. My concern is 
> the pragmatic one of not being able to use ready() as a clean approach to a 
> problem I'm currently solving in a less than ideal way.

In fact, that was part of the original implementation. My main motivation was 
to make the transition easier for pluggable apps, and especially contrib apps. 
That’s also what you’re looking for right now.

>> 3) Do 2, but with a level of indirection. Let the user call the AppConfig 
>> module whatever they want, but allow the apps module to define a 
>> 'default_app_config' attribute that provides the string to use as a default 
>> AppConfig. 
> 
> we could put default_app_config in the application’s root package but that’s 
> ugly.
> 
> I agree that it's less than ideal, but I don't think a string definition in 
> __init__is *more* ugly than implicit imports and registrations happening in 
> the __init__  - in fact, I'd say it's a whole lot *less* ugly, because it's 
> just identifying a string in a module that was already being imported, and 
> using that as a trigger to import another specific module (one that the end 
> user could reference explicitly if they were so inclined).

Indeed, this convention provides a way for pluggable apps to select a default 
AppConfig class without requiring end-users to change INSTALLED_APPS.

It would make the transition much easier. For instance, at the moment, there’s 
no way to configure the debug toolbar in a way that works both on 1.6 and 1.7.

I’m at least +0, and probably +1 on the idea, but I’d like to hear the opinion 
of other people who contributed to the current design, especially Jannis.

-- 
Aymeric.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/45F925BB-699F-4D7D-9871-ACD766E85926%40polytechnique.org.
For more options, visit https://groups.google.com/groups/opt_out.

Re: App-loading: Pragmatic concerns about default AppConfig objects and ready() implementations

[Russell Keith-Magee]

On Sun, Jan 19, 2014 at 9:52 PM, Marc Tamlyn  wrote:

> For what it's worth, if we have some code that:
>
> a) is a new feature we want to ensure runs
> b) has no obvious, explicit place to place it
> and
> c) is trivially solved by .ready()
>
> Then I start to think that practicality beats purity. Personally, I think
> we should still encourage the "explicit" approach where we recommend to
> users that they use 'path.to.AppConfig' and maybe one day deprecate not
> having an AppConfig - for external apps at least this ensures translatable
> admin, but in the mean time add an impure convention (which if we are going
> to do, I don't care which one we do).
>
> Explicit > Implicit yes, but if our choice is 'introduce an implicit
> convention, which we could one day deprecate' vs 'add some more import time
> side effect code' then I personally think we should go for the former.
>
The other option (which was option 1, and seems to have been overlooked)
was to not do anything implicit, and make it a hard upgrade path using the
checks framework itself. We can have compatibility checks that explicitly
look for the contrib app strings in INSTALLED_APPS, and raise an error to
the effect that "your app isn't being fully validated". Core compatibility
checks aren't tied to apps embedded in the system, so they always run, so
you wouldn't be able to do a syncdb/migrate or runserver without hitting an
error that has hint telling you want you need to do. The migration itself
is simple -- modify three lines in your INSTALLED_APPS, and you never see
it again. It's just a question of whether we'd prefer a hands-off migration
using an implicit technique, or a hands-on migration pointed to by the
checks framework.

> In Django 2.0 we do away with all the implicit importing and searching and
> you have to configure your AppConfig instances for management commands,
> template directories, static dirs, models, admin config... ;)
>
Oh, that mystical land of Django 2.0, where the sun shines shiner … :-)

Russ %-)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/CAJxq84_9Gj8hHJxdjESFig7zMkL5p6xx2iHAyp%3Dsz16Xu%3DB%3DvA%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: App-loading: Pragmatic concerns about default AppConfig objects and ready() implementations

[Marc Tamlyn]

For what it's worth, if we have some code that:

a) is a new feature we want to ensure runs
b) has no obvious, explicit place to place it
and
c) is trivially solved by .ready()

Then I start to think that practicality beats purity. Personally, I think
we should still encourage the "explicit" approach where we recommend to
users that they use 'path.to.AppConfig' and maybe one day deprecate not
having an AppConfig - for external apps at least this ensures translatable
admin, but in the mean time add an impure convention (which if we are going
to do, I don't care which one we do).

Explicit > Implicit yes, but if our choice is 'introduce an implicit
convention, which we could one day deprecate' vs 'add some more import time
side effect code' then I personally think we should go for the former.

In Django 2.0 we do away with all the implicit importing and searching and
you have to configure your AppConfig instances for management commands,
template directories, static dirs, models, admin config... ;)

Marc
On 19 Jan 2014 12:52, "Russell Keith-Magee"  wrote:

>
> HI Marc,
>
> On Sun, Jan 19, 2014 at 5:52 PM, Marc Tamlyn wrote:
>
>> All of your proposed solutions require trying to import an apps.py module
>> which may not exist. I know Aymeric was very much against this, especially
>> as python imports have potential side effects, and we don't know what
>> people might have in a apps module already.
>>
>> Anyways, configuration over convention and explicit better than implicit
>> etc.
>>
>> It does mean that we can't put any required logic into AppConfig, but for
>> example admin.autodiscover is fine as existing projects already call it in
>> their urls.py (or wherever they moved it to), so there is no concern here,
>> especially if we update the project template.
>>
>> The only other obvious contrib thing which would be nice to move is
>> signal registration, but this isn't that necessary.
>>
>> The hope is that people will migrate towards using AppConfig in their
>> settings, and then we can reconsider. Third party apps who wish to do a lot
>> of logic there may wish to require it.
>>
>> Personally I'm torn, as I can see the benefits of a convention. But
>> equally I don't think there's a huge amount of point making a big effort to
>> rewrite Django's import handling in order to add more magic back in.
>>
>> See my comments to Aymeric. I'm not blind to the concerns you've raised
> here - there certainly valid ones, and I appreciate (and sympathise with)
> the intent. I'm just stuck in a bind where there's a new feature that will
> fix my problem much better than the existing alternatives -- I just can't
> use it.
>
> I'm completely open to other approaches that don't require speculative
> imports. Aymeric's suggestion of an attribute in the root module sounds
> like a good option to me -- it only requires an attribute access in a
> module that you're already importing, and you use that string to provide
> the value that we'd like to tell the end user to put in their
> INSTALLED_APPS. However, it sounds like Aymeric wasn't as keen on this
> approach.
>
> Yours,
> Russ Magee %-)
>
>  --
> You received this message because you are subscribed to the Google Groups
> "Django developers" 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/CAJxq84_MPKw_zywaeaQ-4aXDYaTVJxfRtm%2B-2GEXrrn7D7VwJQ%40mail.gmail.com
> .
> For more options, visit https://groups.google.com/groups/opt_out.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/CAMwjO1H9i1t7iGCQCHSV-B4GCgMac2CWDYfLxQoo8Zw%2BD_mXvQ%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Feature request: serve_file() view in static app

[German Larrain]

PyPI packages 'static' and 'dj-static' might help you.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/55985ad8-990c-4341-b876-167eb7a1a3fa%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Improving aggregate support (#14030)

[Josh Smeaton]

I've finally sent a PR, so if you're still able, I'd love to see the 
specific comments you had.

https://github.com/django/django/pull/2184

Cheers,

- Josh

On Sunday, 12 January 2014 10:35:58 UTC+11, Michael Manfre wrote:
>
> With minor tweaks to django-mssql's SQLCompiler, I am able to pass the 
> aggregation and aggregation_regress tests. If you create the pull request, 
> comments can be attached to specific lines of code.
>
> I haven't had time to do a full review of your changes, but my initial 
> questions and comments are:
>
> - Why were the aggregate specific fields, properties, etc. added to 
> ExpressionNode, instead of being on django.db.models.aggregates?
> - Why does an ExpressionNode containing any Aggregates need to pollute the 
> entire tree with that knowledge?
> - Instead of using is_ordinal and is_computed (confusing name, why not 
> is_float), you could eliminate those and define output_type for each 
> Aggregate that does something special.
> - What does is_summary mean?
>
> Regards,
> Michael Manfre
>
>
> On Sat, Jan 11, 2014 at 3:06 AM, Josh Smeaton 
>  > wrote:
>
>> A milestone has been reached. The diff is here: 
>> https://github.com/jarshwah/django/compare/akaariai:lookups_3...aggregates-14030?expand=1
>>
>>- Existing annotations and aggregations working as they were
>>- Complex annotations and complex aggregations now work
>>- Complex annotations can now be referenced by an aggregation
>>- Cleaned up the logic relating to aggregating over an annotation
>>
>> See the tests for the new functionality here: 
>> https://github.com/jarshwah/django/blob/aggregates-14030/tests/aggregation/tests.py#L662
>>
>> Things I still need to tackle:
>>
>>- GIS
>>- Custom backend implementation of aggregates and annotations 
>>- Annotations that aren't aggregates. I think this should be 
>>extremely easy, but I might not be aware of some underlying difficulties.
>>
>> Things that should be done in the future, as I don't think there will be 
>> time for this just now:
>>
>>- Change order_by (and other functions) to accept Expressions.
>>
>> I would think that, as it stands, this code is ready for review with the 
>> above caveats. If there are any backend authors reading (mssql, 
>> django-nonrel), I would encourage you to look at the implementation here, 
>> and let me know if there's anything glaringly obvious (to you) that is 
>> missing or broken.
>>
>> Regards,
>>
>> - Josh
>>
>> On Thursday, 9 January 2014 00:33:08 UTC+11, Josh Smeaton wrote:
>>>
>>> I'll preface by saying it's late for me, and I'm about to sign off, so 
>>> any mistakes below I will correct tomorrow after some sleep.
>>>  
>>>
 To write a custom aggregate one needed two classes, one for user facing 
 API, one for implementing that API for SQL queries.
>>>
>>>
>>> I've taken note of this already, and that is the reason I've kept the 
>>> sql.Aggregate class where it is, though it's not used internally anymore. 
>>> There are two options that I can see right now:
>>>
>>> - The non-compatible way involves moving the sql_function attribute to 
>>> the Aggregate from the SQLAggregate, and everything should work fine
>>> - The backward-compatible way might involve re-adding the "add_to_query" 
>>> function, and using the SQLAggregate to monkeypatch the Aggregate by 
>>> copying over the sql_function attribute. I would have to experiment and 
>>> test this though. I assume this would be the more appropriate option. We 
>>> can include a DeprecationWarning to phase it out.
>>>  
>>>
 Another case you should check is how GIS works *without* rewriting 
 anything in django.contrib.gis
>>>
>>>
>>> Good idea, that's how I'll approach the aggregate stuff. Not sure that I 
>>> can leave it completely alone due to Expressions though, but we'll see (and 
>>> hope). 
>>>
>>> I'll try to check your work in detail as soon as possible. Unfortunately 
 I am very busy at the moment, so I can't guarantee anything.
>>>
>>>
>>> Totally understandable, I appreciate your comments regardless. It's just 
>>> unfortunate there aren't more people deeply familiar with the ORM - it's 
>>> bloody complicated :P
>>>
>>> I'm now to the point where all tests in aggregate, aggregate_regress, 
>>> expressions, expressions_regress, and lookup pass. I've got a small problem 
>>> with .count() regressions failing, but am working through that now. 
>>> Tomorrow I should be able to start playing with new functionality and 
>>> writing new tests. I'll also take a look into running tests on GIS and 
>>> fixing any errors that rear their heads there.
>>>
>>> Cheers,
>>>
>>> - Josh
>>>  
>>>
>>> On Thursday, 9 January 2014 00:05:23 UTC+11, Anssi Kääriäinen wrote:

 On Friday, January 3, 2014 6:34:13 PM UTC+2, Josh Smeaton wrote:
>
> I now have all tests passing on Postgres and SQLite (See 
> 

Re: App-loading: Pragmatic concerns about default AppConfig objects and ready() implementations

[Russell Keith-Magee]

HI Marc,

On Sun, Jan 19, 2014 at 5:52 PM, Marc Tamlyn  wrote:

> All of your proposed solutions require trying to import an apps.py module
> which may not exist. I know Aymeric was very much against this, especially
> as python imports have potential side effects, and we don't know what
> people might have in a apps module already.
>
> Anyways, configuration over convention and explicit better than implicit
> etc.
>
> It does mean that we can't put any required logic into AppConfig, but for
> example admin.autodiscover is fine as existing projects already call it in
> their urls.py (or wherever they moved it to), so there is no concern here,
> especially if we update the project template.
>
> The only other obvious contrib thing which would be nice to move is signal
> registration, but this isn't that necessary.
>
> The hope is that people will migrate towards using AppConfig in their
> settings, and then we can reconsider. Third party apps who wish to do a lot
> of logic there may wish to require it.
>
> Personally I'm torn, as I can see the benefits of a convention. But
> equally I don't think there's a huge amount of point making a big effort to
> rewrite Django's import handling in order to add more magic back in.
>
> See my comments to Aymeric. I'm not blind to the concerns you've raised
here - there certainly valid ones, and I appreciate (and sympathise with)
the intent. I'm just stuck in a bind where there's a new feature that will
fix my problem much better than the existing alternatives -- I just can't
use it.

I'm completely open to other approaches that don't require speculative
imports. Aymeric's suggestion of an attribute in the root module sounds
like a good option to me -- it only requires an attribute access in a
module that you're already importing, and you use that string to provide
the value that we'd like to tell the end user to put in their
INSTALLED_APPS. However, it sounds like Aymeric wasn't as keen on this
approach.

Yours,
Russ Magee %-)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/CAJxq84_MPKw_zywaeaQ-4aXDYaTVJxfRtm%2B-2GEXrrn7D7VwJQ%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: App-loading: Pragmatic concerns about default AppConfig objects and ready() implementations

[Russell Keith-Magee]

On Sun, Jan 19, 2014 at 5:48 PM, Aymeric Augustin <
aymeric.augus...@polytechnique.org> wrote:

> On 19 janv. 2014, at 08:56, Russell Keith-Magee 
> wrote:
>
> This is fine, but it limits the logic that we (as a project) can put into
> AdminConfig.ready(), because existing projects won't reference the new
> AdminConfig. So, cleanups like putting admin.autoregister and check
> framework registrations into the ready function can't be done, because
> existing projects won't automatically gain that functionality.
>
> So, we end up with a situation where we have a nifty new on-startup
> method, but we can't use it for any of our own on-startup app requirements,
> because we have no guarantees that it's going to be invoked.
>
>
> I would take a more moderate position.
>
> We can certainly use AppConfig.ready() for new features such as the checks
> framework. It’s reasonable to ask our users to edit their configuration to
> take advantage of a new feature. If they don’t, they don’t lose anything
> compared to the previous version of Django. Backwards-compatibility is
> preserved.
>

I'm afraid I don't see *how* we can use ready() for new features. Checks
are *definitely* off the table for using the feature. Take Auth, for
example. We need to register the various checks related to AUTH_USER_MODEL.
We need to register these checks regardless of whether you're using
django.contrib.auth or django.contrib.auth.apps.AuthApp. So we have to put
the registration somewhere it will be picked up by the
'django.contrib.auth' usage; at which point, any usage in ready() is either
redundant, or requires a logic check along the lines of "if you were added
to INSTALLED APPS using an AppConfig, don't register",

The same goes for admin and contenttypes (the two other contrib apps that
gain checks modules).

We can also use it for optional features such as admin.autodiscover().
> Putting it in urls.py still works, but it’s simply better to use
> AdminConfig. The release notes explain the upgrade path. It users don’t
> follow them, they don’t lose anything.
>

I'll certainly grant this one - auto discover is something that's been
"opt-in" (of a fashion), so we can tell people to just stop using it
(and/or change the project template).


> If we wanted to move a major existing feature to AppConfig.ready(), and
> stop supporting the previous implementation, then we’d have to implement a
> deprecation path or, depending on the context, document it as a
> backwards-incompatible change. But this situation still has to happen.
>

Right - which is effectively a variant on Option 1 - treat this as a "you
need to update" problem, and provide a deprecation path and upgrade
compatibility check as assisting tools.


> As far as I can tell, we’re talking about an hypothetical problem.
>

It's not hypothetical at all - I actually can't implement the checks
framework using the ready() method, because that logic path won't be
executed. I have to use an import side effect in the __init__ module of
contrib.auth, contrib.contenttypes and contrib.admin. This is less than
ideal, but it's not something were we have any other option, AFAICT --
hence my enthusiasm about using ready().

> Assuming that this wasn't done for import-related reasons, I can see a
> couple of possible solutions:
>
> 1) Require updating INSTALLED_APPS as a migration step. This is something
> we can detect with a check migration command - we inspect all the strings
> in INSTALLED_APPS; if it isn't an AppConfig subclass, look to see if there
> is an apps module, and if so, suggest that an update may be required.
>
> Benefits - explicit upgrade path.
> Downside - if you ignore the warning, you potentially lose functionality.
> And nobody ever reads the documentation :-)
>
>
> It really depends how much we want to push the use of AppConfig. At the
> moment the consensus among the core devs I’ve spoken with is “wait and
> see”. That means not making the feature too prominent in 1.7 and adding
> suitable documentation in 1.8, depending on how well it works.
>

I suppose I've been around long enough wishing we had App objects that I
can't wait to see them in active use :-)

Pragmatically, I can appreciate the take-it-slow approach.


> Also there might be good reasons for not using an AppConfig.
>

I'd be interested in hearing what these are. Having a stable and
predictable place for signal registration is worth the price of admission
alone, IMHO. I've got plenty of things that I'd like to start experimenting
with around AppConfig objects.

> 2) Use a naming convention - require that the default app configuration
> must be called AppConfig, so we can always try to load
> myapp.apps.AppConfig. If this object doesn't exist, install the system
> default AppConfig (as it does currently).
>
> Benefits - Easy to implement and document the requirement.
> Downside - It's coding by convention; also, every app config is called
> AppConfig, which is potentially painful for 

Re: Ticket #21751 review requested

[Shai Berger]

On Sunday 19 January 2014 11:52:40 Łukasz Rekucki wrote:
> On 19 January 2014 09:12, Shai Berger  wrote:
> > On Friday 17 January 2014 01:19:29 Michael Manfre wrote:
> >> In an effort to make Django a bit more explicit with regards to closing
> >> cursors when they are no longer needed, I have created ticket #21751 [1]
> >> with a pull request[2].
> >> 
> >> Most of the changes are straight forward and change the usage of a
> >> cursor so that it uses a context manager or closes the cursor in
> >> 'finally'.
> >> 
> >> Example:
> >> 
> >> # old pattern
> >> connection.cursor().execute(...)
> >> 
> >> # changes to
> >> 
> >> with connection.cursor() as cursor:
> >>cursor.execute(...)
> > 
> > I think this is suboptimal, API-wise. Python destroys temporaries soon
> > enough.
> 
> You mean CPython, right? Considering that a DB cursor can also
> allocate resources in the DB itself, I think it's not a good idea to
> rely on GC here. Unfortunately, the pull request in question won't
> help in many cases due to the way how try: finally: works in
> generators[1].
> 
Yep, I had CPython's behavior in mind. Didn't realize how different the others 
were in this regard.

Still, spreading with-blocks all over the code for this looks very ugly to me. 
I'd rather add an execute_single() or execute_and_close() method to the 
cursors instead. Perhaps even only as private API, as that usage is mostly 
common within Django's code.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/201401191223.53778.shai%40platonix.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: App-loading: Pragmatic concerns about default AppConfig objects and ready() implementations

[Marc Tamlyn]

All of your proposed solutions require trying to import an apps.py module
which may not exist. I know Aymeric was very much against this, especially
as python imports have potential side effects, and we don't know what
people might have in a apps module already.

Anyways, configuration over convention and explicit better than implicit
etc.

It does mean that we can't put any required logic into AppConfig, but for
example admin.autodiscover is fine as existing projects already call it in
their urls.py (or wherever they moved it to), so there is no concern here,
especially if we update the project template.

The only other obvious contrib thing which would be nice to move is signal
registration, but this isn't that necessary.

The hope is that people will migrate towards using AppConfig in their
settings, and then we can reconsider. Third party apps who wish to do a lot
of logic there may wish to require it.

Personally I'm torn, as I can see the benefits of a convention. But equally
I don't think there's a huge amount of point making a big effort to rewrite
Django's import handling in order to add more magic back in.

Marc
On 19 Jan 2014 07:56, "Russell Keith-Magee"  wrote:

> Hi all,
>
> First off - this isn't critical for 1.7 alpha - I'm raising it now because
> I've been in this space for the last couple of days as a result of the
> working on the check framework. I suspect any changes stemming from this
> discussion can be landed in the beta period without major problems.
>
> Also - Aymeric - you've done a great job with this app loading work. I
> didn't get much of a chance to follow along as it was unfolding, but the
> resultant work has been a pleasure to work with.
>
> With one exception :-)
>
> We've now got AppConfig objects for each app (e.g., contrib.admin). Those
> AppConfig objects can have ready() methods that invoke startup logic. This
> is all great stuff that has been a long time coming.
>
> *But*
>
> From a backwards compatibility perspective, there seems to be a gap in the
> way AppConfig objects are instantiated.
>
> The gap is this - historical apps (and historical documentation) calls for
> admin to be put into INSTALLED_APPS using 'django.contrib.admin'. However,
> when you do this, you get the system default AppConfig, not something that
> is admin specific. In order to get the new AdminConfig, you need to update
> your INSTALLED_APPS to point at "django.contrib.admin.apps.AdminConfig".
>
> This is fine, but it limits the logic that we (as a project) can put into
> AdminConfig.ready(), because existing projects won't reference the new
> AdminConfig. So, cleanups like putting admin.autoregister and check
> framework registrations into the ready function can't be done, because
> existing projects won't automatically gain that functionality.
>
> So, we end up with a situation where we have a nifty new on-startup
> method, but we can't use it for any of our own on-startup app requirements,
> because we have no guarantees that it's going to be invoked.
>
> I've dug through the mailing lists discussions about app-loading, and I
> can't see anywhere that this was explicitly discussed, so I can't tell if
> this was done deliberately, or as a side effect of the circular import
> problems that Aymeric described.
>
> Assuming that this wasn't done for import-related reasons, I can see a
> couple of possible solutions:
>
> 1) Require updating INSTALLED_APPS as a migration step. This is something
> we can detect with a check migration command - we inspect all the strings
> in INSTALLED_APPS; if it isn't an AppConfig subclass, look to see if there
> is an apps module, and if so, suggest that an update may be required.
>
> Benefits - explicit upgrade path.
> Downside - if you ignore the warning, you potentially lose functionality.
> And nobody ever reads the documentation :-)
>
> 2) Use a naming convention - require that the default app configuration
> must be called AppConfig, so we can always try to load
> myapp.apps.AppConfig. If this object doesn't exist, install the system
> default AppConfig (as it does currently).
>
> Benefits - Easy to implement and document the requirement.
> Downside - It's coding by convention; also, every app config is called
> AppConfig, which is potentially painful for debugging.
>
> 3) Do 2, but with a level of indirection. Let the user call the AppConfig
> module whatever they want, but allow the apps module to define a
> 'default_app_config' attribute that provides the string to use as a default
> AppConfig.
>
> This is essentially halfway between 1 and 2 - it's implicitly doing the
> INSTALLED_APPS update, using information provided by the app developer.
>
> 3a) As for 3, but use a flag on the AppConfig subclass that marks it as
> "use me as the default". If there are more than one AppConfig objects in an
> apps module that has the 'use as default" flag, raise an error.
>
> 3b) As for 3, but use the first class discovered that is a 

Re: Ticket #21751 review requested

[Łukasz Rekucki]

On 19 January 2014 09:12, Shai Berger  wrote:
> On Friday 17 January 2014 01:19:29 Michael Manfre wrote:
>> In an effort to make Django a bit more explicit with regards to closing
>> cursors when they are no longer needed, I have created ticket #21751 [1]
>> with a pull request[2].
>>
>> Most of the changes are straight forward and change the usage of a cursor
>> so that it uses a context manager or closes the cursor in 'finally'.
>>
>> Example:
>>
>> # old pattern
>> connection.cursor().execute(...)
>>
>> # changes to
>> with connection.cursor() as cursor:
>>cursor.execute(...)
>>
>
> I think this is suboptimal, API-wise. Python destroys temporaries soon enough.

You mean CPython, right? Considering that a DB cursor can also
allocate resources in the DB itself, I think it's not a good idea to
rely on GC here. Unfortunately, the pull request in question won't
help in many cases due to the way how try: finally: works in
generators[1].

> Is there a reason why we cannot arrange for a __del__ method to close the
> cursor? Circular references anywhere?

AFAIK, this is already the case as long as the wrapped cursor does it.

[1]: https://bugs.pypy.org/issue736

-- 
Łukasz Rekucki

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/CAEZs-EKQJAfZpoOq30mRy%2B_PxOGt%3DoFLt4H7W0CaNjyY%3DbpXqA%40mail.gmail.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Ticket #21751 review requested

[Aymeric Augustin]

On 19 janv. 2014, at 09:12, Shai Berger  wrote:

> I think this is suboptimal, API-wise. Python destroys temporaries soon 
> enough. 
> Is there a reason why we cannot arrange for a __del__ method to close the 
> cursor? Circular references anywhere?

Hi Shai,

You probably meant CPython ;-) I believe this change will improve consistency 
with other Python implementations, notably PyPy.

Following your reasoning, we should also stop bothering about closing files, as 
Python will do it. But that's a bad practice and Python 3.3+ even raises 
warnings in that situation. Is there a difference between file descriptors and 
cursors (in practice TCP sockets) that justifies treating them differently? 
(Honest question — I don’t know.)

-- 
Aymeric.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/C9435FD7-412A-41DA-9E13-90264B812B94%40polytechnique.org.
For more options, visit https://groups.google.com/groups/opt_out.

Re: App-loading: Pragmatic concerns about default AppConfig objects and ready() implementations

[Aymeric Augustin]

On 19 janv. 2014, at 08:56, Russell Keith-Magee  wrote:

> This is fine, but it limits the logic that we (as a project) can put into 
> AdminConfig.ready(), because existing projects won't reference the new 
> AdminConfig. So, cleanups like putting admin.autoregister and check framework 
> registrations into the ready function can't be done, because existing 
> projects won't automatically gain that functionality. 
> 
> So, we end up with a situation where we have a nifty new on-startup method, 
> but we can't use it for any of our own on-startup app requirements, because 
> we have no guarantees that it's going to be invoked.

I would take a more moderate position.

We can certainly use AppConfig.ready() for new features such as the checks 
framework. It’s reasonable to ask our users to edit their configuration to take 
advantage of a new feature. If they don’t, they don’t lose anything compared to 
the previous version of Django. Backwards-compatibility is preserved.

We can also use it for optional features such as admin.autodiscover(). Putting 
it in urls.py still works, but it’s simply better to use AdminConfig. The 
release notes explain the upgrade path. It users don’t follow them, they don’t 
lose anything.

If we wanted to move a major existing feature to AppConfig.ready(), and stop 
supporting the previous implementation, then we’d have to implement a 
deprecation path or, depending on the context, document it as a 
backwards-incompatible change. But this situation still has to happen.

As far as I can tell, we’re talking about an hypothetical problem.

> Assuming that this wasn't done for import-related reasons, I can see a couple 
> of possible solutions:
> 
> 1) Require updating INSTALLED_APPS as a migration step. This is something we 
> can detect with a check migration command - we inspect all the strings in 
> INSTALLED_APPS; if it isn't an AppConfig subclass, look to see if there is an 
> apps module, and if so, suggest that an update may be required. 
> 
> Benefits - explicit upgrade path.
> Downside - if you ignore the warning, you potentially lose functionality. And 
> nobody ever reads the documentation :-)

It really depends how much we want to push the use of AppConfig. At the moment 
the consensus among the core devs I’ve spoken with is “wait and see”. That 
means not making the feature too prominent in 1.7 and adding suitable 
documentation in 1.8, depending on how well it works.

Also there might be good reasons for not using an AppConfig.

> 2) Use a naming convention - require that the default app configuration must 
> be called AppConfig, so we can always try to load myapp.apps.AppConfig. If 
> this object doesn't exist, install the system default AppConfig (as it does 
> currently). 
> 
> Benefits - Easy to implement and document the requirement.
> Downside - It's coding by convention; also, every app config is called 
> AppConfig, which is potentially painful for debugging.

Jannis vetoes that idea and I believe he’s right. Auto-importing modules for 
discovery is a bad idea. “One does not simply import modules.” ;-)

> 3) Do 2, but with a level of indirection. Let the user call the AppConfig 
> module whatever they want, but allow the apps module to define a 
> 'default_app_config' attribute that provides the string to use as a default 
> AppConfig. 
> 
> This is essentially halfway between 1 and 2 - it's implicitly doing the 
> INSTALLED_APPS update, using information provided by the app developer.
> 
> 3a) As for 3, but use a flag on the AppConfig subclass that marks it as "use 
> me as the default". If there are more than one AppConfig objects in an apps 
> module that has the 'use as default" flag, raise an error.
> 
> 3b) As for 3, but use the first class discovered that is a subclass of 
> AppConfig. Actually identifying the "first" is a bit of an implementation 
> issue - I suppose we'd need to have a MetaClass that registered AppConfig 
> objects as they are imported. 


All these still suffer from the implicit import problem. I chose not to perform 
any kind of discovery because explicit > implicit

To avoid that, we could put default_app_config in the application’s root 
package but that’s ugly.

-- 
Aymeric.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/CA318055-63E5-43EC-9F5F-A9A6E326C381%40polytechnique.org.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Ticket #21751 review requested

[Shai Berger]

On Friday 17 January 2014 01:19:29 Michael Manfre wrote:
> In an effort to make Django a bit more explicit with regards to closing
> cursors when they are no longer needed, I have created ticket #21751 [1]
> with a pull request[2].
> 
> Most of the changes are straight forward and change the usage of a cursor
> so that it uses a context manager or closes the cursor in 'finally'.
> 
> Example:
> 
> # old pattern
> connection.cursor().execute(...)
> 
> # changes to
> with connection.cursor() as cursor:
>cursor.execute(...)
> 

I think this is suboptimal, API-wise. Python destroys temporaries soon enough. 
Is there a reason why we cannot arrange for a __del__ method to close the 
cursor? Circular references anywhere?

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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/201401191012.46962.shai%40platonix.com.
For more options, visit https://groups.google.com/groups/opt_out.