Re: #3011 - Custom User Models -- Call for final review

2012-09-18 Thread Donald Stufft 
On Tuesday, September 18, 2012 at 10:34 PM, Ben Slavin wrote:
> Those apps that require (or choose to offer) a deeper stack of version 
> support can choose to do so, but the pragmatism of making the common case 
> easy (and removing the need for cross-project duplication) seems to justify 
> the unified interface. 
After I sent that I thought about it more, adding explicit values in 1.4.x 
doesn't really hurt anything. And the example wrapper functions could still be 
added to the docs for people who need more. 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #3011 - Custom User Models -- Call for final review

2012-09-18 Thread Ben Slavin 

On Tuesday, September 18, 2012 9:34:38 PM UTC-4, dstufft wrote:
>
> On Tuesday, September 18, 2012 at 9:13 PM, Ben Slavin wrote:
>
> Lastly, I haven't seen a path to easily allow third-party apps to 
> gracefully support both The Old Way and The New Way (1.4 and 1.5). It feels 
> a bit wrong, but should we be considering  the addition of get_user_model 
> and settings.AUTH_USER_MODEL to 1.4.x that's hardcoded to contrib.auth.User 
> so third-party apps can rely on the presence of these mechanisms?
>
>  I don't think adding a hardcoded AUTH_USER_MODEL is going to give the 
> kind of coverage people would want. Likely the best way would be for people 
> to write a wrapper function around the relevant methods/settings and 
> include it in their own projects. This sucks for duplication across all 
> those projects but otherwise it means they'll only be able to support the 
> latest 1.4.x and 1.5+. Likely to be a better user compatibility story by 
> handling the fallback on a per app basis (Django could provide examples 
> though).
>

I agree that this does limit things to only 1.4.X and 1.5+, but that seems 
acceptable. I did consider the possibility of not adding this support 
layer, but after consideration still felt it will justify the effort.

Those apps that require (or choose to offer) a deeper stack of version 
support can choose to do so, but the pragmatism of making the common case 
easy (and removing the need for cross-project duplication) seems to justify 
the unified interface.

Not speaking as a matter of official policy, but running the latest micro 
version of the previous minor version seems a reasonable requirement for 
third-party app developers to impose.

 - Ben

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/dxid-t-e9LUJ.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #3011 - Custom User Models -- Call for final review

2012-09-18 Thread Ben Slavin 

>
> If your project specifies a custom user, you should get validation 
> warnings saying that there are foreign keys to a swapped model (and 
> indicate which foreign keys are affected). 
>

Indeed I did. This was greatly appreciated.
 

> This does means that there is a "1.5 compatible" barrier to entry for 
> apps. However, the barrier is clearly marked and validated, it only 
> matters if you want to use the 1.5-specific "custom User" feature, and 
> the migration path should be fairly simple. The update verges on being 
> fixable with a regex, but not quite, since you really do need to 
> engage a brain in the process to make sure you haven't got any implied 
> User interface contracts.
>

You're right that my concern overlooks the case where reliance on the User 
goes beyond a mere ForeignKey, but I've found that in many cases usage is 
restricted to this simple interface.

Maybe this is just a case of "sometimes life sucks", but I felt like I was 
wasting bits by forking a handful of packages for testing purposes only to 
change one line per project. I don't believe that there's a universal 
solution here and I'll respect that the current decision correctly chooses 
one pain over another.
 

> Yes, this does mean we need to work with the community. The 1.5 
> release notes do mention this, but would warrant making a bit more 
> noise about this when we do the release -- possibly a "If you read 
> nothing else, read this" section of the release notes.


My concern here is that we're creating a situation where "if you try to use 
custom User models on day 1, don't plan to use third party packages that 
don't have proactive management". As above, this may be unavoidable, but 
it's unfortunate.
 

> > An especially large problem has been with South. Existing migrations 
> will 
> > explicitly refer to auth.User. That is to say: a fresh installation on 
> the 
> > initial `./manage.py migrate` will behave incorrectly because foreign 
> keys 
> > will be pointing to an incorrect model. I inquired over on #django-south 
> and 
> > didn't get any response on any possible plans to handle the new use 
> cases 
> > introduced by the t3011 changes. Not a core problem, but something we 
> should 
> > work with the community on to ensure a smooth path. 
>
> Agreed - migrations are going to be a problem. That's why the docs 
> currently say "get this right before you sync". I'm open to 
> suggestions on how to make this more prominent/obvious. 
>

I don't think the problem here is one of obviousness. I think there are 
unaddressed problems here that 'get it right before you sync' trivializes.

Let's take django-activity-stream for 
example: 
https://github.com/justquick/django-activity-stream/blob/master/actstream/migrations/0001_initial.py#L14

In this migration, South has created an explicit reference to auth.User. 
The only two options I see are: 1) fork and manually patch the migrations 
to point to a custom user model or 2) abandon migrations (./manage.py 
syncdb --all && ./manage.py migrate fake)

To be clear, the majority of this burden lies outside of Django (at least 
until we merge the migration branch). I tried to start the dialogue with 
the South community today, but I think we're well advised to make sure that 
community is aware of the issues this can create.


As we've reviewed this, aside from the patch mentioned in my previous post, 
we have found no technical issues. The only concerns that remain from our 
team are focused on pragmatism and minimizing the burden on external 
developers.

Best,
   - Ben

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/8yFCOK0TylEJ.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #3011 - Custom User Models -- Call for final review

2012-09-18 Thread Russell Keith-Magee 
On Wed, Sep 19, 2012 at 9:13 AM, Ben Slavin  wrote:
> Hi Russ,
>
> First, let me apologize for being a bit late to the party on this. If
> there's been prior discussion of any of the points below kindly tell me to
> get stuffed and so shall I do.

You asked for it… :-)

> Our team has been working with the t3011 branch today.
>
> We ran into some trouble running tests locally. The tests were in our own
> project (not django), but BaseDatabaseIntrospection.sequence_list() was
> listing sequences for all models, and not taking into account that some
> might be swappable. Josh Ourisman threw together a patch to fix this. Russ:
> You should have seen a pull request, but the diff is available at
> https://github.com/joshourisman/django/commit/ef46bbb5520499ebcda2e074446d2c4a055dd6e8

I've seen the pull request. It looks good on first inspection; lI'll
try and find some time tonight to merge it in.

> We've run into quite a few issues with third party packages that have
> foreign keys to auth.User. Fixing this may be outside the scope of Django
> proper, but it will require work with the community. To enable our testing,
> we've had to fork quite a few apps.

Ok - this is how the migration path *should* work (at least, this is
how it was planned to work):

If your project is using the default User, nothing changes. Third
party apps have an explicit foreign key to User, and they continue to
work.

If your project specifies a custom user, you should get validation
warnings saying that there are foreign keys to a swapped model (and
indicate which foreign keys are affected).

This does means that there is a "1.5 compatible" barrier to entry for
apps. However, the barrier is clearly marked and validated, it only
matters if you want to use the 1.5-specific "custom User" feature, and
the migration path should be fairly simple. The update verges on being
fixable with a regex, but not quite, since you really do need to
engage a brain in the process to make sure you haven't got any implied
User interface contracts.

Yes, this does mean we need to work with the community. The 1.5
release notes do mention this, but would warrant making a bit more
noise about this when we do the release -- possibly a "If you read
nothing else, read this" section of the release notes.

> An especially large problem has been with South. Existing migrations will
> explicitly refer to auth.User. That is to say: a fresh installation on the
> initial `./manage.py migrate` will behave incorrectly because foreign keys
> will be pointing to an incorrect model. I inquired over on #django-south and
> didn't get any response on any possible plans to handle the new use cases
> introduced by the t3011 changes. Not a core problem, but something we should
> work with the community on to ensure a smooth path.

Agreed - migrations are going to be a problem. That's why the docs
currently say "get this right before you sync". I'm open to
suggestions on how to make this more prominent/obvious.

> Lastly, I haven't seen a path to easily allow third-party apps to gracefully
> support both The Old Way and The New Way (1.4 and 1.5). It feels a bit
> wrong, but should we be considering  the addition of get_user_model and
> settings.AUTH_USER_MODEL to 1.4.x that's hardcoded to contrib.auth.User so
> third-party apps can rely on the presence of these mechanisms?

This is a good suggestion, and one that has some precedent with other
features we've introduced in the past.

> All of this aside, this is great work. This has been an itch I've longed to
> scratch for quite some time, and this work does it more completely and more
> correctly than my efforts in django-samaritan. Cheers to you, Russ, and to
> everyone who's been part of forming this solution over the last six years!

You're most welcome. Like you've said, my contribution is really only
the last little part -- this has been a work in various forms of
progress for a long time, and I've certainly benefited from the
experiences of others.

Russ %-)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #3011 - Custom User Models -- Call for final review

2012-09-18 Thread Donald Stufft 
On Tuesday, September 18, 2012 at 9:13 PM, Ben Slavin wrote:
> Lastly, I haven't seen a path to easily allow third-party apps to gracefully 
> support both The Old Way and The New Way (1.4 and 1.5). It feels a bit wrong, 
> but should we be considering  the addition of get_user_model and 
> settings.AUTH_USER_MODEL to 1.4.x that's hardcoded to contrib.auth.User so 
> third-party apps can rely on the presence of these mechanisms?
> 
I don't think adding a hardcoded AUTH_USER_MODEL is going to give the kind of 
coverage people would want. Likely the best way would be for people to write a 
wrapper function around the relevant methods/settings and include it in their 
own projects. This sucks for duplication across all those projects but 
otherwise it means they'll only be able to support the latest 1.4.x and 1.5+. 
Likely to be a better user compatibility story by handling the fallback on a 
per app basis (Django could provide examples though). 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: #3011 - Custom User Models -- Call for final review

2012-09-18 Thread Ben Slavin 
Hi Russ,

First, let me apologize for being a bit late to the party on this. If 
there's been prior discussion of any of the points below kindly tell me to 
get stuffed and so shall I do.


Our team has been working with the t3011 branch today.

We ran into some trouble running tests locally. The tests were in our own 
project (not django), but BaseDatabaseIntrospection.sequence_list() was 
listing sequences for all models, and not taking into account that some 
might be swappable. Josh Ourisman threw together a patch to fix this. Russ: 
You should have seen a pull request, but the diff is available 
at 
https://github.com/joshourisman/django/commit/ef46bbb5520499ebcda2e074446d2c4a055dd6e8

We've run into quite a few issues with third party packages that have 
foreign keys to auth.User. Fixing this may be outside the scope of Django 
proper, but it will require work with the community. To enable our testing, 
we've had to fork quite a few apps.

An especially large problem has been with South. Existing migrations will 
explicitly refer to auth.User. That is to say: a fresh installation on the 
initial `./manage.py migrate` will behave incorrectly because foreign keys 
will be pointing to an incorrect model. I inquired over on #django-south 
and didn't get any response on any possible plans to handle the new use 
cases introduced by the t3011 changes. Not a core problem, but something we 
should work with the community on to ensure a smooth path.

Lastly, I haven't seen a path to easily allow third-party apps to 
gracefully support both The Old Way and The New Way (1.4 and 1.5). It feels 
a bit wrong, but should we be considering  the addition of get_user_model 
and settings.AUTH_USER_MODEL to 1.4.x that's hardcoded to contrib.auth.User 
so third-party apps can rely on the presence of these mechanisms?


All of this aside, this is great work. This has been an itch I've longed to 
scratch for quite some time, and this work does it more completely and more 
correctly than my efforts in django-samaritan. Cheers to you, Russ, and to 
everyone who's been part of forming this solution over the last six years!


Best,
 - Ben

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/BoV0niJnMQkJ.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Results of testing votizen's py2.6/7 codebase against django master (319e1355190)

2012-09-18 Thread Jeremy Dunck 
On Tue, Sep 18, 2012 at 11:23 AM, Jeremy Dunck  wrote:
...
> And the last one, I hesitate to raise because it's likely to be
> specific to my machine, but... our (django-nose-based) test runner
> hangs after completing the suite but before tearing down.  Using
> dtruss I can see it's hanging on select/kevent, and using gdb I see
> it's in _mysql_ConnectionObject_query.  I'm unsure where to get debug
> symbols for either mac python or mysqldb (or at least, I haven't spent
> the time to get there).  I mention it despite it seeming
> environment-specific because it's possibly hanging due to the
> unicode/py3 changes.  I just don't know.
>
> #0  0x7fff9867aaf2 in read ()
> #1  0x0001105fe8ba in vio_read ()
> #2  0x00011057fbb1 in my_real_read ()
> #3  0x00011057f778 in my_net_read ()
> #4  0x000110572cb2 in cli_safe_read ()
> #5  0x00011057705d in cli_read_query_result ()
> #6  0x00011057608e in mysql_real_query ()
> #7  0x000110563d14 in _mysql_ConnectionObject_query ()
> #8  0x00010f738d77 in PyEval_EvalFrameEx ()

Florian and Anssi helped troubleshoot this -
https://code.djangoproject.com/ticket/18984

I am using multi-db with some TEST_MIRROR'd aliases, each of which has
a pending transaction.  The flush at the end of TestCase locks forever
waiting for a table lock, essentially deadlocking.  Rolling back
pending transactions just before flush releases the related locks.

I consider this a release blocker because many multidb apps would
likely hit this.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Results of testing votizen's py2.6/7 codebase against django master (319e1355190)

2012-09-18 Thread Jeremy Dunck 
In response to the call to test py2.x on django's current master, I
ran the test suite for Votizen.  Our codebase is currently running
with django 1.4.x.

Our codebase is ~100kloc, ~32kloc of which is tests.  We use abstract
model inheritance a good bit, but no concrete inheritance.  No i18n or
multi-tz.

In use:
  python2.6/2.7
  python-memcache
  tastypie
  MySQLdb
  django-celery/celery
  django-imagekit
  django-classy-tags

Results (so far):
  1) Our custom test runner broke: we used to force DB connection
feature detection (there was a bug in TEST_MIRROR'd feature detection)
so we could do some juggling; forced detection is now gone and is
implicit; this seems to remove the need to even do the underlying hack
for us.

  2) Revealed a DB alias which was missing a TEST_MIRROR declaration
for us.  It used to work, but we were wrong and just lucky.

  3) tastypie broken by a const moving in the django tree; fixed in
https://github.com/toastdriven/django-tastypie/commit/71a1f0a161a7f3d549bce9b07984212ee4a8c81a

  4) after upgrading tastypie master
(27d17b7db7afd6c81da24f64f5b4562b59134582), a bunch of failures like:
...
return self.dispatch('list', request, **kwargs)
  vi +176  upvote/api/tastypie.py  # dispatch
response = self._dispatch(request_type, request, **kwargs)
...
  vi +172  upvote/api/tastypie.py  # _dispatch
request, **kwargs)
  vi +474  
/Users/jdunck/.virtualenvs/votizen-1.5/lib/python2.7/site-packages/tastypie/resources.py
 # dispatch
response = method(request, **kwargs)
  vi +1130 
/Users/jdunck/.virtualenvs/votizen-1.5/lib/python2.7/site-packages/tastypie/resources.py
 # get_list
paginator = self._meta.paginator_class(request.GET,
sorted_objects, resource_uri=self.get_resource_uri(),
limit=self._meta.limit, max_limit=self._meta.max_limit,
collection_name=self._meta.collection_name)
TypeError: get_resource_uri() takes exactly 2 arguments (1 given)

-- it turns out our subclass .get_resource_uri *required*
bundle_or_obj, while tastypie internally does bundle_or_obj=None.  It
happened to work on 0.9.11; switching ours to also be optional fixed.

  5) We have some URLNode subclasses and as such had to have a
compatibility layer for future.url and furture.URLNode moving to
django.template.

  6) We had some test failures where
TransactionalTestCase.assertContains used to work (text was coerced by
smart_str prior to d1452f60974da6f0e54ff9ad7a03d2c115675d10).  While
we could argue that only text vars should be passed to
assert(Not)Contains, I think the removal of smart_str(text) here was
likely unintentional.  Post-py3ization, I think it should probably be
put back as force_text(text, response._charset).
I raised a ticket and made a related patch/pull req:
https://code.djangoproject.com/ticket/18980


I have a couple remaining issues which I've run out of time to work
through just now:

upvote.api.tests.test_tastypie:VoterResourceTestCase.test_X
ValueError: astimezone() cannot be applied to a naive datetime
(tastypie master + dj 1.5?)

upvote.candidates.tests.views.test_profile_home:CandidatesProfileTest.test_friends
EmptyPage: That page contains no results
(almost certainly an internal problem)

And the last one, I hesitate to raise because it's likely to be
specific to my machine, but... our (django-nose-based) test runner
hangs after completing the suite but before tearing down.  Using
dtruss I can see it's hanging on select/kevent, and using gdb I see
it's in _mysql_ConnectionObject_query.  I'm unsure where to get debug
symbols for either mac python or mysqldb (or at least, I haven't spent
the time to get there).  I mention it despite it seeming
environment-specific because it's possibly hanging due to the
unicode/py3 changes.  I just don't know.

#0  0x7fff9867aaf2 in read ()
#1  0x0001105fe8ba in vio_read ()
#2  0x00011057fbb1 in my_real_read ()
#3  0x00011057f778 in my_net_read ()
#4  0x000110572cb2 in cli_safe_read ()
#5  0x00011057705d in cli_read_query_result ()
#6  0x00011057608e in mysql_real_query ()
#7  0x000110563d14 in _mysql_ConnectionObject_query ()
#8  0x00010f738d77 in PyEval_EvalFrameEx ()

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Schema Alteration - Review needed!

2012-09-18 Thread Andrew Godwin 
An update from discussions with Alex and Anssi - I'm going to modify things
a little so we don't have a Borg-pattern AppCache (i.e. you can instantiate
it multiple times and get different caches), which should solve most of the
problems currently caused by app cache state fiddling. Should take a day or
two.

I can help with Oracle problems if you encounter a such thing since that's
> my main backend I work with. (And I have to deal with all the pain it
> causes)


Thanks Jani - that would be much appreciated. It should be possible to get
a decent way just from the Oracle module, I'll let you know when I get
around to it.

Andrew

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.