Re: Requiring sqlparse for sqlite introspection

2018-11-04 Thread Adam Johnson
If you can see a way to improve those docs submit a PR

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

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

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

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

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

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

easy pcikings

2018-11-04 Thread abyk476
Hi i am Abhith,iam new to django and i have read the newcomers guide.
I found an open ticket under new pickings.But i dont know what to do next
Can someone pls guide me.
thanks,

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


Re: easy pcikings

2018-11-04 Thread Carlton Gibson
Hi Abhith. 

Have a look at the Triage Workflow 
docs: 
https://docs.djangoproject.com/en/dev/internals/contributing/triaging-tickets/#triage-stages

For each stage of the ticket's lifecycle they give you what's next. (e.g. 
if it's "New", can you reproduce the issue so it can move to "Accepted", if 
"Accepted", does it have a patch, and can you add one. And so on.) 

If seems complicated but mostly it's just "Is this a real issue?" and "Can 
I fix it?" 

Assuming it can be reproduced, creating a Unit Test is the next best 
step: 
https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/unit-tests/#unit-tests
Often, once there's a failing test, the fix won't be far away. 

Make sure you read the Advice for New 
Contributors: 
https://docs.djangoproject.com/en/dev/internals/contributing/new-contributors/

And if you get stuck, email 
django-core-mentorship: 
https://groups.google.com/forum/#!forum/django-core-mentorship 
— that's the place for help with contributing. 
Make sure you include as much details as you can (e.g. ticket number, where 
you've got to and any errors you're seeing etc) — the more detail you can 
provide the easier it is to help you. 

I hope that gets you started, and welcome aboard! 

Kind Regards,

Carlton


On Sunday, 4 November 2018 14:53:07 UTC+1, aby...@gmail.com wrote:
>
> Hi i am Abhith,iam new to django and i have read the newcomers guide.
> I found an open ticket under new pickings.But i dont know what to do next
> Can someone pls guide me.
> thanks,
>

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


Explore integrating django-docker-box in some way?

2018-11-04 Thread Tom Forbes
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/CAFNZOJNFvp5ke_SzZtto2JEy%3DUsUSWFHByf-sA_ahUW9Vb_Brg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


Re: Remove pyinotify autoreloader support

2018-11-04 Thread Tom Forbes
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 (charett...@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/pull/8819) and I’ve made some good
> progress but I am worried about the lack of tests for pyinotify (there are
> none!). The current code in master works but it’s really hard to refactor
> in any meaningful way without them. I would very much like to explore
> adding support for watchdog (https://pythonhosted.org/watchdog/) instead
> of pyinotify and these changes I’m working on are a means to that end.
>
> Wwatchdog is a library that wraps efficient platform-specific filesystem
> notifications in cross-platform way, and it includes an inotify
> implementation.
>
> So I think there are three ways forward:
>
>1.
>
>Spend time and effort adding special tests for PyInotify (testing this
>stuff is not simple, especially with the current code)
>2.
>
>Remove the functionality with an eye to using something like watchdog
>in the near future
>3.
>
>Never really touch the autoreloader much (it is some of the oldest and
>nastiest code we have).
>
> Does anyone have any strong opinions on this? Does anyone use the
> pyinotify speedup while developing?
>
> T