Fellow Reports -- November 2018

2018-11-05 Thread Carlton Gibson


Hi all,



Calendar Week 44 -- ending 04 November.




Released: Django v2.1.3




Triaged:


https://code.djangoproject.com/ticket/29917 -- admin.E130 (__name__ 
uniqueness) regression (needsinfo)

https://code.djangoproject.com/ticket/29905 -- Can you fix ticket 26171 in 
1.8.x (wontfix)




Reviewed:


https://code.djangoproject.com/ticket/29917 -- Admin actions are duplicated 
when using model admins with inheritance

https://code.djangoproject.com/ticket/29891 -- Fix incorrect quoting in 
queryset.query

https://code.djangoproject.com/ticket/29615 -- Document difference in 
behaviour between m2m_changed behaviour for add() vs remove() when called 
multiple times.

https://code.djangoproject.com/ticket/29868 -- Database constraint checks 
are not retained on sqlite




Authored:


https://github.com/django/django/pull/10607 -- Fixed #29917 — Adjusted MRO 
collection of ModelAdmin.actions.




Kind Regards,


Carlton

-- 
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/d947b603-027d-496c-bac2-fe3212d13fb1%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Proposal to remove ModelAdmin's collection of actions from superclasses

2018-11-05 Thread Tim Graham
Hi,

A recent bug report [1] brought up the fact that ModelAdmin collects 
actions from superclasses. For example:

class BaseAdmin:
actions = ['a']

class SubAdmin(BaseAdmin):
actions = ['b']

SubAdmin will have action 'a' and 'b'. The behavior isn't tested and only 
mentioned in passing in docs for ModelAdmin.get_action(), "Most of the time 
you’ll use this method to conditionally remove actions from the list 
gathered by the superclass."

I think the reason for the "collect from superclasses" behavior was to 
inherit the "delete_selected" action from BaseModelAdmin, however, that 
reason was obsoleted in a later commit where AdminSite actions were added 
and delete_selected was moved there.

I propose removing this surprising (to me, at least) behavior and follow 
normal Python inheritance. If someone wants to inherit actions from a 
subclass, they should use:

class SubAdmin(BaseAdmin):
actions = BaseAdmin.actions + ['b']

[1] https://code.djangoproject.com/ticket/29917
[2] 
https://github.com/django/django/commit/bb15cee58a43eeb0d060f8a31f9078b3406f195a

-- 
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/3235fe9c-30e2-4534-93eb-c7f6a45eac63%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Proposal to remove ModelAdmin's collection of actions from superclasses

2018-11-05 Thread Adam Johnson
I agree, it shouldn’t be doing something so surprising and undocumented.

On Mon, 5 Nov 2018 at 17:03, Tim Graham  wrote:

> Hi,
>
> A recent bug report [1] brought up the fact that ModelAdmin collects
> actions from superclasses. For example:
>
> class BaseAdmin:
> actions = ['a']
>
> class SubAdmin(BaseAdmin):
> actions = ['b']
>
> SubAdmin will have action 'a' and 'b'. The behavior isn't tested and only
> mentioned in passing in docs for ModelAdmin.get_action(), "Most of the time
> you’ll use this method to conditionally remove actions from the list
> gathered by the superclass."
>
> I think the reason for the "collect from superclasses" behavior was to
> inherit the "delete_selected" action from BaseModelAdmin, however, that
> reason was obsoleted in a later commit where AdminSite actions were added
> and delete_selected was moved there.
>
> I propose removing this surprising (to me, at least) behavior and follow
> normal Python inheritance. If someone wants to inherit actions from a
> subclass, they should use:
>
> class SubAdmin(BaseAdmin):
> actions = BaseAdmin.actions + ['b']
>
> [1] https://code.djangoproject.com/ticket/29917
> [2]
> https://github.com/django/django/commit/bb15cee58a43eeb0d060f8a31f9078b3406f195a
>
> --
> 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/3235fe9c-30e2-4534-93eb-c7f6a45eac63%40googlegroups.com
> 
> .
> For more options, visit https://groups.google.com/d/optout.
>
-- 
Adam

-- 
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/CAMyDDM3hPSFtsVVGkJVann25BaXe60LGuxcVw04%2B0MVp7LYKZw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Proposal to remove ModelAdmin's collection of actions from superclasses

2018-11-05 Thread Carlton Gibson
The idea was that you could define a base admin and have the actions appear 
without having to redeclare them. 

The only question is are people actually using that? (I can't think of a 
third-party app that provides an admin with actions that you're meant to 
subclass for instance — anyone?) 

If not, then the "usual Python behaviour" argument is favourite. (i.e. +1) 

— Carlton

-- 
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/6314b21b-5be9-4ab3-ba9d-7e246d796e4b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Proposal to remove ModelAdmin's collection of actions from superclasses

2018-11-05 Thread charettes
I agree with you Tim. +1 from me.

Le lundi 5 novembre 2018 12:03:55 UTC-5, Tim Graham a écrit :
>
> Hi,
>
> A recent bug report [1] brought up the fact that ModelAdmin collects 
> actions from superclasses. For example:
>
> class BaseAdmin:
> actions = ['a']
>
> class SubAdmin(BaseAdmin):
> actions = ['b']
>
> SubAdmin will have action 'a' and 'b'. The behavior isn't tested and only 
> mentioned in passing in docs for ModelAdmin.get_action(), "Most of the time 
> you’ll use this method to conditionally remove actions from the list 
> gathered by the superclass."
>
> I think the reason for the "collect from superclasses" behavior was to 
> inherit the "delete_selected" action from BaseModelAdmin, however, that 
> reason was obsoleted in a later commit where AdminSite actions were added 
> and delete_selected was moved there.
>
> I propose removing this surprising (to me, at least) behavior and follow 
> normal Python inheritance. If someone wants to inherit actions from a 
> subclass, they should use:
>
> class SubAdmin(BaseAdmin):
> actions = BaseAdmin.actions + ['b']
>
> [1] https://code.djangoproject.com/ticket/29917
> [2] 
> https://github.com/django/django/commit/bb15cee58a43eeb0d060f8a31f9078b3406f195a
>

-- 
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/46ac7697-035f-4b56-8bea-8a1d054287e3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Proposal: relationships based on arbitrary predicates

2018-11-05 Thread charettes
Hello Alex,

While investigating an issue with reverse known related objects 
assignment[0] I stumbled
upon a little something that might interest you.

It looks like there's currently some tested cases of abusing the 
ForeignObject interface
to achieve something similar to what you are suggesting here[1][2].

The contenttypes.GenericRelation object happens to implement a similar 
interface
while it's actually a many-to-many field. I assume it's not setting 
many_to_many=True
because a ton of internal Django checks duck-type this attribute to figure 
whether or
not "through" exists but it should really be marked this way.

I'd say giving a shot at reimplementing GenericRelation on top of your 
suggested
Relationship object would be a good way to test whether or not it's 
flexible enough
and be a strong argument for inclusion in Django core to get rid of the 
current hacks
GenericRelation currently employs.

Simon

[0] https://code.djangoproject.com/ticket/29908
[1] 
https://github.com/django/django/blob/ecac6d7a2a510cb0a5d772deeca633c99c9687e5/tests/foreign_object/models/empty_join.py#L22-L84
[2] 
https://github.com/django/django/blob/ecac6d7a2a510cb0a5d772deeca633c99c9687e5/tests/foreign_object/models/empty_join.py#L98-L103

Le mardi 18 septembre 2018 21:43:56 UTC-4, Alex Hill a écrit :
>
> Hi Silvio,
>
> Thanks for your feedback. David Sanders brought up something similar to 
> ComputedField on GitHub[0] - a ForwardedField which can traverse 
> relationships and present a field from a remote instance as if it were a 
> local field. As long as it can traverse relationships, that use case would 
> be covered by ComputedField, so that's another tick for that idea.
>
> I think these two features are quite distinct - except that they may both 
> require a similar change to the ORM to allow traversing relations in the 
> referred-to fields. One limitation of relativity now is that you can only 
> form conditions with local fields of your models. I'd like to be able to 
> traverse relationships in those conditions.
>
> Anybody who's interested - please try out relativity and see if it works 
> for you. The mptt and treebeard helpers are a good place to start :)
>
> Simon - any further thoughts on this before I start working up a patch?
>
> Thanks,
> Alex
>
> [0] 
> https://github.com/AlexHill/django-relativity/pull/1#issuecomment-418239406
>
> On Mon, 10 Sep 2018 at 4:59 am, Silvio > 
> wrote:
>
>> Alex,
>>
>> This is a very useful pattern, that like many others, I've also 
>> implemented in an ad-hoc fashion using a ton of undocumented internal APIs. 
>> So I fully agree standardizing it would be great. Something similar is:
>>
>> https://groups.google.com/forum/#!topic/django-developers/ADSuUUuZp3Q
>>
>> Essentially, I've ended up with the need for:
>>
>> ComputedField
>>
>> and
>>
>> ComputedRelationship
>>
>> where both have all of the niceties that regular fields and foreign 
>> relationships have.
>>
>> So I'd love to see this in Django.
>>
>> -
>> Silvio
>>
>>
>> On Sunday, September 2, 2018 at 10:55:58 PM UTC-4, Alex Hill wrote:
>>>
>>> Hi Simon,
>>>
>>> Thanks for looking at this and for providing some context - I had looked 
>>> at FilteredRelation but I hadn't seen reverse-unique. It makes me more 
>>> confident that this is a good direction to take. I've reimplemented 
>>> ReverseUnique using Relationship [0] and the tests pass, with the only code 
>>> carried over that for discovery of the FK link.
>>>
>>> > I'm not totally sold on the API but having an analog of what 
>>> ForeignObject is to ForeignKey for ManyToManyField would definitely be 
>>> useful.
>>>
>>> I'm not tied to the API, but I think passing a Q as a predicate makes 
>>> sense especially given that it's what both FilteredRelation and 
>>> ReverseUnique do. The core of the idea is that we can express a 
>>> relationship as a combination of predicate and arity. In practise I don't 
>>> think this would be used all that much by users directly - more by 
>>> third-party apps like mptt, and perhaps Django internally.
>>>
>>> > From what I can see in relativity.fields[0] most of the additional 
>>> logic revolves around the extra filtering capabilites through Restriction.
>>>
>>> Yeah that's what it boils down to. We return no columns to join against, 
>>> and return a compilable Restriction from get_extra_restriction to provide 
>>> all the ON conditions. The rest of it is making the descriptors, rels, 
>>> prefetch, etc work.
>>>
>>> > Do you have an idea of what the fields.related inheritance chain would 
>>> look like if it was part of core?
>>>
>>> The least intrusive, and probably a good starting point, would be to 
>>> introduce Relationship alongside the other relation fields as a standalone 
>>> feature, modifying the ORM to allow the implementation to be less hacky. It 
>>> would remain a subclass of ForeignObject (or perhaps RelatedField - I'll 
>>> give that a try).
>>>
>>> In the future there's potential for a nice refactor o

Re: Idea: Allow queryset.get() and queryset.filter() to accept a positional argument for implicit primary key filtering

2018-11-05 Thread Josh Smeaton
I'm in the same boat as Simon - I've wanted this many times in the last few 
months, but only when working at the shell. I'd be +1 for get and -1 for 
filter also.

On Thursday, 1 November 2018 05:12:53 UTC+11, charettes wrote:
>
> As I've mentioned on the ticket I've been wishing get(pk) could translate
> to get(pk=pk) for a while but I never got to suggest it. Probably because I
> didn't feel like adding (pk=...) was really that long. I'd note that most 
> of the
> times I wished this existed was when doing some object manipulations
> through a Django shell.
>
> I'm not convinced this would be as useful for `filter()` though as I don't
> recall wanting to retrieve a set of objects matching a value I know will
> be unique.
>
> Something to keep in mind as well is whether or not we want to allow
> this to be coupled with extra args and kwargs.
>
> e.g.
>
> Book.objects.get(isbn, author__name='Daniel Roy Greenfeld')
>
> I'd be in favor of preventing pk and kwarg or Q args mixing.
>
> Count me +1 for the get() case and -1 for the filter() one.
>
> Simon
>
> Le mercredi 31 octobre 2018 13:13:34 UTC-4, Antwan a écrit :
>>
>> Hi, 
>> I'm creating this topic to see if there is interest to implement 
>> positional arguments in queryset filtering.
>>
>> Current situation 
>>
>> Currently the only way to use positional arguments to filter can be 
>> either:
>>
>>- Passing a single or multiple Q objects: 
>>
>>MyClass.objects.filter(Q(key=value))
>>MyClass.objects.filter(Q(key=value), Q(other_key=value))
>>
>>
>>
>>- Passing a couple is also working (not sure if this is a happy 
>>accident, should it be removed?) 
>>
>>MyClass.objects.filter((key, value))
>>
>>
>>
>>- Combination of both is also proven to work 
>>
>>MyClass.objects.filter((key, value), Q(other_key=value))
>>
>>
>>
>> Suggestion 
>>
>> This feature suggestion is to leverage the case when a non-Q / non couple 
>> object is passed, so it implicitly interpreted as the value for the model's 
>> pk.
>>
>> This could ease/simplify code by omitting pk when this is the only 
>> filter used:
>>
>>
>> MyClass.objects.get(value)
>> # Translates into: MyClass.objects.get(pk=value)
>>
>>
>> or
>>
>>
>> MyClass.objects.filter(value)
>> # Translates into: MyClass.objects.filter(pk=value)
>>
>>
>> or 
>>
>>
>> MyClass.objects.filter(Q(value))
>> # Translates into: MyClass.objects.filter(Q(pk=value))
>>  
>>
>> Do you think it's worth it? It could be leveraged to simplify many 
>> situations.
>> I'd be happy to proceed to the development myself if this is something 
>> gathering interest.
>>
>

-- 
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/ba9128d7-9e49-4269-b3a3-9995a998b491%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: backend specific tests

2018-11-05 Thread Josh Smeaton
I don't think there's a full list of extensions the test suite uses, 
but https://docs.djangoproject.com/en/2.1/ref/contrib/postgres/operations/ 
would be close to a full set I'd imagine.

On Sunday, 4 November 2018 14:43:23 UTC+11, Dan Davis wrote:
>
> So, the contributor guidelines page about unit tests mentions running 
> database specific tests:
>
>
> https://docs.djangoproject.com/en/2.1/internals/contributing/writing-code/unit-tests/#testing-other-python-versions-and-database-backends
>
> I am working on ticket 29984, and it seems to me that since the TruncDay 
> is attempting to cast to the timezone on the database level, it is working, 
> and its job is done.  So, the fix should be at the backend level, and the 
> ticket provides one for Postgres.
>
> Following the unit test advice, I created a settings file.   But not using 
> my Postgresql admin user.   I get the following errors:
>
> psycopg2.ProgrammingError: permission denied to create extension 
> "btree_gin"
> HINT:  Must be superuser to create this extension.
>
> I'm not going to do that - do you have any list of required extensions?
>

-- 
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/d6b9f379-3a3d-4ea7-925c-ddd3f2422dab%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Explore integrating django-docker-box in some way?

2018-11-05 Thread Josh Smeaton
I'm sorry I haven't had the time to review or contribute yet, but I think 
it'll be a very useful project - especially for new contributors that might 
have a little docker experience. The current vagrant solution is heavy, 
does not work properly on windows and some linuxes, and isn't that easy to 
maintain or deploy. I'd be in favour of adding the docker files directly to 
django/django to minimise setup burden (DJANGO_PATH), and improving the 
contributing docs to show users how to test using docker.

One of the hardest things I found at sprints was getting development 
environments setup to effectively contribute - even using the docker-box 
project which I understand quite well. Anything we can do to improve that 
situation will be very beneficial.

I have fewer opinions about the official CI story, hopefully some of the 
infrastructure team can comment more on that. I think that replacing the 
ansible roles with a docker setup can have some definite improvements and 
open up CI tasks to a larger pool of people (anyone that can edit docker 
files), but it'd come with maintaining the host that runs docker (cleaning 
up images, dealing with disk space issues, etc).


On Monday, 5 November 2018 01:20:03 UTC+11, Tom Forbes wrote:
>
> Hello all,
>
> I’ve been working on a docker-compose based alternative to django-box 
> (imaginatively named django-docker-box) over the last month and it 
> finally appears to be mostly complete. 
>
> For reference the tool is just a Dockerfile and a docker-compose 
> definition that is able to run a complete test matrix of every supported 
> Python and DB version. It’s as simple as docker-compose run sqlite. You 
> can see a full test run (excluding oracle) here: 
> https://travis-ci.com/orf/django-docker-box/builds/90167436
>
> Florian suggested I create a thread here to gather feedback and discuss 
> any potential future directions for the project, so here goes:
>
> Firstly I’d like to know if there is any support for moving this under the 
> Django project itself, maybe even as a replacement for django-box? I think 
> the setup is pretty quick compared to django-box and is more flexible in 
> terms of database version support as well as working with Oracle. I’d also 
> really like some help improving Oracle support if anyone has the time!
>
> Secondly is there any support for integrating this with our current 
> Jenkins setup? I think it would be pretty neat to have parity between what 
> runs on the CI and what we can run locally and have any improvements shared 
> between both. Perhaps a full matrix run (which right now is 66 different 
> environments) is out of the question but a smaller subset could be good?
>
> Thirdly, and this is a bit wild, but what about using this to reduce the 
> burden of running Jenkins by running the tests on a managed CI service like 
> Travis CI? We would likely still need Jenkins due to issues with Oracle and 
> running tests on Windows (unless 
> https://github.com/django/django/pull/10259 works with Docker!), but we 
> could offload some of the environments onto a third party service. Travis 
> gives large OS projects like Django increased concurrency limits on their 
> accounts so we could end up with pretty speedy test runs. Also with 
> docker-compose switching between CI services (including Jenkins) would be 
> very simple.
>
> The repo is here: https://github.com/orf/django-docker-box.
>
> Any feedback on these points or the project itself would be greatly 
> appreciated,
>
> Tom 
>
>
>

-- 
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/074f68c9-2199-4128-a37a-bfc1852f4806%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Remove pyinotify autoreloader support

2018-11-05 Thread Josh Smeaton
I ran a slightly modified version of your test code since we have many 
modules at "top level". "k3" is one of the top level modules that houses a 
bunch of apps. In total there are about 1340 python files (including 
recently squashed migrations and tests). Here's the results:

Project: 1575
Total: 7381
[(u'django', 890),
 (u'k3', 675),
 (u'nltk', 426),
 (u'newrelic', 294),
 (u'google', 172),
 (u'braintree', 171),
 (u'requests', 144),
 (u'rest_framework', 108),
 (u'boto', 101),
 (u'suds', 95),
 (u'numpy', 94),
 (u'core', 90),
 (u'weasyprint', 87),
 (u'PIL', 87),
 (u'docutils', 73),
 (u'cssutils', 70),
 (u'hyper', 68),
 (u'Crypto', 68),
 (u'email', 62),
 (u'oauthlib', 56),
 (u'cryptography', 55),
 (u'pygments', 55),
 (u'celery', 54),
 (u'oauth2client', 53),
 (u'elasticsearch', 50),
 (u'coreapi', 48),
 (u'jinja2', 47),
 ]

On Monday, 5 November 2018 02:14:12 UTC+11, Tom Forbes wrote:
>
> I have done a lot more reading on this and I really feel pyinotify may not 
> be required, or we could at least switch to a watchman based service for 
> simplicity right away. Projects like uwsgi use a stat based approach, as 
> do most other projects I’ve seen and it appears to work ok for them. 
> watchman has the advantage of being easier to test, as we can mock the 
> actual watchman service itself, and it supplies a sane, simple API across 
> all supported platforms. However it suffers from some limitations I need to 
> work around.
>
> As an example of a fairly large project I took a look at Sentry. After 
> running the test suite (and excluding test modules) there are 1,000 loaded 
> Python modules under sentry.* and 4,000 total entries in sys.modules. 
> Django accounts for about 600, by far the biggest third party module.
>
> During development it seems likely that it’s not worth polling any builtin 
> python modules at all, and drastically reducing the polling time for site 
> packages including Django. That would reduce the number of files to poll by 
> 75%. Perhaps we could even just skip watching site-packages entirely - I 
> think it’s not very common to edit these in a usual development session, 
> and if the user finds themselves editing them we could add a flag to 
> include them in the watch list?
>
> If anyone here has a fairly large Django app they work on could I request 
> that they run this snippet at some point after it has booted and send me 
> the output, to see if Sentry is representative?
>
> from collections import Counter
> counter = Counter(m.split('.')[0] for m in sys.modules.keys())
> total = sum(counter.values())
> project_total = counter['PROJECT NAME HERE']
> print('Project:', project_total)
> print('Total:', total)
> print(counter.most_common(20))
>
> Thanks,
>
> Tom
>
>
>
>
> On 8 October 2018 at 12:04:08, Tom Forbes (t...@tomforb.es ) 
> wrote:
>
> Thanks for the feedback! In the pull request I have re-added support for 
> pyinotify with tests, it was not as hard to write them as I believed. They 
> still fail, but I’m working on that!
>
> I’ve found an interesting module confusingly called watchgod. The author 
> says[1] that with the new os.scandir() method added in python 3.5 a stat 
> based method can scan a project of 850 files in about 24ms.
>
> While this is watching a single directory tree and not the entirety of 
> sys.modules I believe we could get similar speedups by being a bit more 
> intelligent in our approach. For example we really don’t need to stat every 
> single django.* module every second. I’m guessing that those change very 
> rarely (unless you’re hacking on Django!), so could maybe stat them every 
> 2–3 seconds or even not at all. We could reduce the stating time on 
> site-packages modules as well for similar reasons.
>
> If we do this right we could potentially get the overhead down to an 
> acceptable level that we don’t necessarily need platform specific watchers.
>
>1. 
>
> https://github.com/samuelcolvin/watchgod#why-no-inotify–kqueue–fsevent–winapi-support
>  
>
>
>
> On 6 October 2018 at 20:56:17, charettes (chare...@gmail.com ) 
> wrote:
>
> While I understand the complexity of 1. I think shipping a version of 
> Django without
> an equivalent inotify replacement such as watchdog could be problematic.
>
> From my personal experience when using Django's development server in 
> Docker
> containers sharing local volumes installing pyinotify resulted a 
> significant performance
> CPU and I/O improvement.
>
> Simon
>
> Le samedi 6 octobre 2018 15:32:33 UTC-4, Tom Forbes a écrit : 
>>
>> What do we think about removing the pyinotify functionality in the 
>> autoreloader? For context, if pyinotify is installed (Linux only) the 
>> autoreloader will attempt to use it to detect file changes over the 
>> standard polling-based approach. It is generally much more efficient but is 
>> not cross platform, and is not well documented currently IMO.
>>
>> I’m hacking away at my attempt at refactoring the autoreloader (
>> https://github.com/django/django/p

Re: Standalone is_safe_url() function

2018-11-05 Thread 'Ivan Anishchuk' via Django developers (Contributions to Django itself)
Yeah, it can be pretty useful at times, for example, in api clients. I used
it quite a few times and had no idea it's not a part of the public api.

Ivan.

On Sun, Oct 28, 2018 at 12:29 PM Adam Johnson  wrote:

> I needed that functionality on another project that doesn't use Django at
>> all.
>>
>
> If this was my own project, I would have installed Django and imported the
> function. Afaict it doesn't depend on settings or any other setup so it
> should work from an import. The only concern would be size of site-packages
> but that's rarely an issue with web projects. Was there something else that
> stopped this?
>
> - If so, should Django possibly depend on that package by itself? Given
>> how often Django had security releases because of issues in `is_safe_url()`
>> releasing a smaller package and not the full Django package could possibly
>> be beneficial.
>
>
> This seems like the start of a broader request for unbundling other things
> from Django - if we unbundle this one function, shouldn't we npm-style
> unbundle "all the things"? (e.g. cached-property has been
> 'unbundled'/forked by PyDanny, Django could start depending on that
> version...). But unbundling increases maintenance work, testing complexity,
> and the number of things users need to understand.
>
> Also, I tried to look up documentation for `is_safe_url()` and failed. Is
>> there any particular reason why it's not documented? You can see seven
>> other things in
>> https://docs.djangoproject.com/en/2.1/ref/utils/#module-django.utils.http but
>> not a single mention of is_safe_url...
>>
>
> Being undocumented, is_safe_url isn't part of Django's public API. It
> could probably be documented now at least, given its utility outside of the
> places Django uses it.
>
> On Wed, 10 Oct 2018 at 18:09, ivan via Django developers (Contributions to
> Django itself)  wrote:
>
>> Hi Markus,
>>
>> Well, adding new requirements for every function might be not a scalable
>> approach, but if some stuff in django important for security was separated
>> into a sub-project it could allow more people to patch their projects more
>> easily. From what I've seen in various projects I worked in, far too many
>> folks don't update django (and other deps) often enough because their fear
>> that something will break (and it often does when custom code depending on
>> django internals is involved), keeping some parts of the django codebase in
>> a separate but official package could make it easier for many projects to
>> update just it without worrying, provided that package's api is absolutely
>> stable and backward-compatible. People who run unsupported versions of
>> django (I don't approve but it's a reality we have to deal with) could also
>> benefit from it as they can update the package regardless of which version
>> of django itself they use.
>>
>> The major downsides, it seems to me, are that people might get even
>> sloppier with updating django if they think that security sub-package is
>> enough to stay safe (it should be made clear that it's not) and that it
>> adds an additional dependency (but I think it's ok as long as it's just one
>> and its purpose and contents is obvious). Also it would be hard to decide
>> what to leave in django and what to separate.
>>
>> tl;dr what I'm saying is I'm not against moving it from django to a
>> separate package django will depend on, but I'd prefer it having a broader
>> goal than one function, however important.
>>
>> Also, I tried to look up documentation for `is_safe_url()` and failed. Is
>> there any particular reason why it's not documented? You can see seven
>> other things in
>> https://docs.djangoproject.com/en/2.1/ref/utils/#module-django.utils.http
>> but not a single mention of is_safe_url...
>>
>> Ivan.
>>
>> On Wednesday, October 10, 2018 at 1:06:46 PM UTC+3, Markus Holtermann
>> wrote:
>>>
>>> Hi all,
>>>
>>> Django provides a function `django.utils.is_safe_url()` to ensure that a
>>> given URL (absolute or relative) is safe to redirect to. I needed that
>>> functionality on another project that doesn't use Django at all. I thus
>>> built a standalone is-safe-url Python package that can be installed from
>>> PyPI and exposes exactly that functionality:
>>>
>>>   $ pip install is-safe-url
>>>   Collecting is-safe-url
>>> Downloading https://files.pythonhosted.org/packages/7a/c3
>>>  
>>> /40c363bc4c3d0ddcda3489239ba64752b8c18cb6493e058f8f1b73154925/is_safe_url-1.0-py3-none-any.whl
>>>
>>>   Installing collected packages: is-safe-url
>>>   Successfully installed is-safe-url-1.0
>>>
>>> The code is available on GitLab: https://gitlab.com/MarkusH/is_safe_url
>>>
>>> I'd love to get some feedback on a couple of things:
>>>
>>> - As Django is published under the BSD-3 clause license, the standalone
>>> package is published under the same license. I'd love some feedback if the
>>> package adheres to the required references and naming of the source.
>>>
>>> - I added a note that security issues shou

Re: Idea: Allow queryset.get() and queryset.filter() to accept a positional argument for implicit primary key filtering

2018-11-05 Thread Tom Forbes
I feel this would be a good addition to just .get(), I’ve wanted this while
working with the shell. Model.objects.get(pk) feels very natural to me, and
the common Model.objects.get(pk=pk) always felt overly verbose.




On 5 November 2018 at 22:37:52, Josh Smeaton (josh.smea...@gmail.com) wrote:

I'm in the same boat as Simon - I've wanted this many times in the last few
months, but only when working at the shell. I'd be +1 for get and -1 for
filter also.

On Thursday, 1 November 2018 05:12:53 UTC+11, charettes wrote:
>
> As I've mentioned on the ticket I've been wishing get(pk) could translate
> to get(pk=pk) for a while but I never got to suggest it. Probably because I
> didn't feel like adding (pk=...) was really that long. I'd note that most
> of the
> times I wished this existed was when doing some object manipulations
> through a Django shell.
>
> I'm not convinced this would be as useful for `filter()` though as I don't
> recall wanting to retrieve a set of objects matching a value I know will
> be unique.
>
> Something to keep in mind as well is whether or not we want to allow
> this to be coupled with extra args and kwargs.
>
> e.g.
>
> Book.objects.get(isbn, author__name='Daniel Roy Greenfeld')
>
> I'd be in favor of preventing pk and kwarg or Q args mixing.
>
> Count me +1 for the get() case and -1 for the filter() one.
>
> Simon
>
> Le mercredi 31 octobre 2018 13:13:34 UTC-4, Antwan a écrit :
>>
>> Hi,
>> I'm creating this topic to see if there is interest to implement
>> positional arguments in queryset filtering.
>>
>> Current situation
>>
>> Currently the only way to use positional arguments to filter can be
>> either:
>>
>>- Passing a single or multiple Q objects:
>>
>>MyClass.objects.filter(Q(key=value))
>>MyClass.objects.filter(Q(key=value), Q(other_key=value))
>>
>>
>>
>>- Passing a couple is also working (not sure if this is a happy
>>accident, should it be removed?)
>>
>>MyClass.objects.filter((key, value))
>>
>>
>>
>>- Combination of both is also proven to work
>>
>>MyClass.objects.filter((key, value), Q(other_key=value))
>>
>>
>>
>> Suggestion
>>
>> This feature suggestion is to leverage the case when a non-Q / non couple
>> object is passed, so it implicitly interpreted as the value for the model's
>> pk.
>>
>> This could ease/simplify code by omitting pk when this is the only
>> filter used:
>>
>>
>> MyClass.objects.get(value)
>> # Translates into: MyClass.objects.get(pk=value)
>>
>>
>> or
>>
>>
>> MyClass.objects.filter(value)
>> # Translates into: MyClass.objects.filter(pk=value)
>>
>>
>> or
>>
>>
>> MyClass.objects.filter(Q(value))
>> # Translates into: MyClass.objects.filter(Q(pk=value))
>>
>> Do you think it's worth it? It could be leveraged to simplify many
>> situations.
>> I'd be happy to proceed to the development myself if this is something
>> gathering interest.
>>
> --
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/ba9128d7-9e49-4269-b3a3-9995a998b491%40googlegroups.com

.
For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAFNZOJP8Ny6gYqQ9ssmCHjK_J_uEfe%2BGFTmcy0BA1fPJPEfQig%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Make `raw_id_fields` the default functionality in the admin

2018-11-05 Thread 'Ivan Anishchuk' via Django developers (Contributions to Django itself)
Well, I did personally encounter this issue more than a couple times on
several projects. In most cases simply switching to raw id or read only
where required works fine however it does add additional maintenance
overhead for projects that started small and at some point database grows
and some admin pages stop working. I even had to write some custom code
once when a client wanted to keep the default select interface but there
were too many of those selects with the same options for some objects
(different number of inlines IIRC), I basically added some js code to make
sure the generation and transmission of the list only takes place once. Ah,
another part of the problem was that by default every single object is
iterated and passed to force_text (I think) and that was costly and there
wasn't an obvious way to use, like, a combination of model fields instead.
Maybe some internals got changed recently and it got better though, I
didn't review these parts of django code in a while.

That being said, I do see a value in using the default select sometimes
(aforementioned small configuration tables are a good example) and I did
see a lot of frustration when people see otherwise nice interface failing
miserably just because some new user accounts or other data was added. I'm
not sure forcing raw id is the right way to handle this but doing something
that works by default (i.e. without you having to write a custom admin
class or even form) regardless of table size would be a good idea. How
about some field that intelligently guesses the table size and looks either
like default select or like raw id field depending on that? A COUNT(*)
query is usually much less costly than fetching the whole table and doing
things with it. Additional (and simple enough) optimization could be done
by making sure options for the same queryset on multiple fields or formset
forms are only generated once, maybe we could cache them somewhere in the
modelform/formset objects?

Also, although it might be obvious and even documented somewhere that
__str__() method shouldn't perform additional db queries if you expect to
use string coercion on a bunch of objects (actually I don't know if it is
documented at all, I just hope so, correct me if I'm wrong), maybe some
simple way to override string coercion method for the options and in
similar places where a bunch of objects has to be converted to a list of
strings could be a good idea (possibly on model/queryset level, some
attribute or method that modelform would pick up and use to make lighter
values() query instead of the default fetching, instantiation, coercion).
I'm not sure what particular solution would be the best here though or I
would've proposed one already, but it definitely shouldn't however be
complicated or require much customization (if you need something
complicated and customized you can always customize forms and fields and it
stops being a problem), it should be something simple and effective for
when you prefer to use the default classes wherever possible.

Ivan.

On Sun, Oct 21, 2018 at 10:48 AM Aymeric Augustin <
aymeric.augus...@polytechnique.org> wrote:

> Hello,
>
> The default  widget is fine for configuration tables with no more
> than a few dozen values.
>
> In my experience, I rarely encountered use cases for editing foreign keys
> to very large tables in the admin.
>
> Readonly or autocomplete are good choices.
>
> Best regards,
>
> --
> Aymeric.
>
>
>
> On 17 Oct 2018, at 17:59, Yo-Yo Ma  wrote:
>
> I have yet to ever come across a situation where the default 
> field is more useful than the raw ID field, pertaining foreign key fields
> in the admin.
>
> I have, however, personally witnessed a major publishing company bring
> their production app servers to a halt (out of memory) due to Django
> attempting to generate 2.5 million  tags for some dozen
> admins that were all refreshing an admin changeview, wondering why it was
> taking so long to load.
>
> Another thing worth noting is that when the  is most useful (when
> there are very few records to select) also happens to be when the raw ID
> field is most easily used (since the selection changelist only contains the
> same very few records). IOW, the raw ID field's usefulness is universal,
> working well with just a few records, and also working well (due to search
> / sort) when there are many records.
>
> Nary a transition would be required, since the `raw_id_fields` could
> simply be ignored.
>
> A new `select_fields = []` could be added for those who wish to easily
> use the old functionality.
>
> Is there any reason why this couldn't or shouldn't be done?
>
>
> --
> 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.c

django.utils.timezone.make_aware confusion

2018-11-05 Thread Dan Davis
Working on ticket #29984, I noticed that the following raises:

from datetime import datetime
import pytz
from django.utils.timezone import make_aware

euberlin = pytz.timezone('Europe/Berlin')s a
dt1 = datetime(2018, 10, 24, tzinfo=euberlin)
dt2 = make_aware(datetime(2018, 10, 24))
assert dt1 == dt2


It is certainly necessary to have make_aware, because datetime may come in 
without a timezone.  I'm familiar with the various solutions - user picks a 
timezone in their profile, and/or JavaScript detects the timezone in the 
browser and provides that to the backend at some point.  I'm even urging a 
coworker to fix his public Django webapp to implement the latter solution.

However, it is disturbing that the datetimes are not equal.   Can someone 
explain why not?

-- 
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/46a382ef-2b19-4b81-87b8-babb1f93b9cc%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: django.utils.timezone.make_aware confusion

2018-11-05 Thread Aymeric Augustin
Hello Dan,

If that assertion fails, I assume the default time zone (the TIME_ZONE setting) 
isn't Europe/Berlin.

See 
https://docs.djangoproject.com/en/2.1/ref/utils/#django.utils.timezone.make_aware
 


If you change it to dt2 = make_aware(datetime(2018, 10, 24), euberlin), it 
should work.

Best regards,

-- 
Aymeric.



> On 6 Nov 2018, at 06:21, Dan Davis  wrote:
> 
> Working on ticket #29984, I noticed that the following raises:
> 
> from datetime import datetime
> import pytz
> from django.utils.timezone import make_aware
> 
> euberlin = pytz.timezone('Europe/Berlin')s a
> dt1 = datetime(2018, 10, 24, tzinfo=euberlin)
> dt2 = make_aware(datetime(2018, 10, 24))
> assert dt1 == dt2
> 
> It is certainly necessary to have make_aware, because datetime may come in 
> without a timezone.  I'm familiar with the various solutions - user picks a 
> timezone in their profile, and/or JavaScript detects the timezone in the 
> browser and provides that to the backend at some point.  I'm even urging a 
> coworker to fix his public Django webapp to implement the latter solution.
> 
> However, it is disturbing that the datetimes are not equal.   Can someone 
> explain why not?
> 
> -- 
> 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/46a382ef-2b19-4b81-87b8-babb1f93b9cc%40googlegroups.com
>  
> .
> For more options, visit https://groups.google.com/d/optout 
> .

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/4EDAF400-ECC6-403C-85D4-8C9D9872BC90%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.


Re: Make `raw_id_fields` the default functionality in the admin

2018-11-05 Thread Aymeric Augustin
On 6 Nov 2018, at 02:47, 'Ivan Anishchuk' via Django developers (Contributions 
to Django itself)  wrote:

> How about some field that intelligently guesses the table size and looks 
> either like default select or like raw id field depending on that?


Yes, that would be interesting.

I implemented a manual version of this in a project:
- I ran a script to count the number of instances of each model and put those 
with more than 100 or 1000 instances in a list
- I wrote a custom ModelAdmin class that automatically added ForeignKeys to 
models in that list to raw_id_fields

Best regards,

-- 
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/EF75EAC1-229E-47B0-ABBB-9C45598D933F%40polytechnique.org.
For more options, visit https://groups.google.com/d/optout.


Re: Idea: Allow queryset.get() and queryset.filter() to accept a positional argument for implicit primary key filtering

2018-11-05 Thread James Bennett
I still don't understand the problem this is solving; is typing "pk=" (or
"id=") that much of a burden? Or that frequently left out by accident?

As it stands, I agree with Adam that this adds implementation complexity
(including potential future implementation complexity, as Ivan noted) and
proliferates different ways to do the same thing, without presenting much
in the way of concrete arguments for why it's needed. If there's a really
convincing case to be made for this, I'm open to reading it when it's made,
but for now I'd be -1 on the whole thing.

-- 
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/CAL13Cg9ocm_LmZSGbQG%3DEB2U0Z%2BOF5eupJFwK3OWbyz46JDj5Q%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.