Re: [Django] #29303: non_atomic_requests decorator alters _non_atomic_requests attribute of original function

[Django]

#29303: non_atomic_requests decorator alters _non_atomic_requests attribute of
original function
-+-
 Reporter:  Alasdair Nicol   |Owner:  Windson
 |  yang
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Alasdair Nicol):

 * status:  closed => new
 * resolution:  needsinfo =>


Comment:

 Replying to [comment:5 Windson yang]:
 > I don't think this is a bug, in your example wrapped_view is my_view.
 You can use id() to find out.

 That explains why the test case is failing, but it's not clear that that
 should be the behaviour. Why should `wrapped_view` be `my_view`?

 If you wrap a view with `transaction.atomic()`, you get a new view:

 {{{
 def my_view(request):
 return HttpResponse('Hello, world')
 wrapped_view = transaction.atomic(my_view)
 assert wrapped_view is not my_view
 }}}

 But when you wrap a view with `transaction.non_atomic_requests`, it
 modifies the original view.

 {{{
 def my_view(request):
 return HttpResponse('Hello, world')
 wrapped_view = transaction.non_atomic_requests(my_view)
 assert wrapped_view is my_view
 }}}

 This behaviour is inconsistent.

 As I said in the original ticket, I'm not sure that this is a problem in
 practice. I assume that most of the time, `non_atomic_requests` is used as
 a decorator, so you don't need the original unwrapped view.

 {{{
 @transaction.non_atomic_requests(using='default')
 @transaction.non_atomic_requests(using='other')
 def my_view(request)
 return HttpResponse('')
 }}}

 I'd understand if we closed this ticket as WONTFIX or simply added a note
 to the documentation, but I think that closing as NEEDSINFO is incorrect.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/071.39eacbc26d486b6bac6198071459f026%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #29303: non_atomic_requests decorator alters _non_atomic_requests attribute of original function

[Django]

#29303: non_atomic_requests decorator alters _non_atomic_requests attribute of
original function
-+-
 Reporter:  Alasdair Nicol   |Owner:  Windson
 |  yang
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:  needsinfo
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Windson yang):

 * status:  assigned => closed
 * resolution:   => needsinfo


Comment:

 I don't think this is a bug, in your example wrapped_view is my_view. You
 can use id() to find out.


 {{{
 class TestViews(TestCase):

 def test(self):
 assert my_view._non_atomic_requests == {'default'}  # passes
 wrapped_view =
 transaction.non_atomic_requests(using='other')(my_view)
 print(id(wrapped_view)) # 4510701904
 print(id(my_view)) # 4510701904
 assert wrapped_view._non_atomic_requests == {'default', 'other'}
 # passes
 assert my_view._non_atomic_requests == {'default'}  # fails
 }}}

 If you want to define a new view, you should use


 {{{
 wrapped_view =
 transaction.non_atomic_requests(using='other')(wrapped_view)
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/071.3d5a79fc0906853c44a431227610aeef%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #29303: non_atomic_requests decorator alters _non_atomic_requests attribute of original function

[Django]

#29303: non_atomic_requests decorator alters _non_atomic_requests attribute of
original function
-+-
 Reporter:  Alasdair Nicol   |Owner:  Windson
 |  yang
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Windson yang):

 * status:  new => assigned
 * owner:  nobody => Windson yang


Comment:

 I'm not sure how to fix this, as we can see, in _non_atomic_requests
 method we add 'using' in the original view then return it. That is why
 my_view._non_atomic_requests changed. The first solution will copy a new
 view from the old one and return the new view. The second will be we make
 the document more clear about this behavior.

 {{{
 def _non_atomic_requests(view, using):
 try:
 view._non_atomic_requests.add(using)
 except AttributeError:
 view._non_atomic_requests = {using}
 return view


 def non_atomic_requests(using=None):
 if callable(using):
 return _non_atomic_requests(using, DEFAULT_DB_ALIAS)
 else:
 if using is None:
 using = DEFAULT_DB_ALIAS
 return lambda view: _non_atomic_requests(view, using)
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/071.7ab1474ba65ab87be0e56ada2b8093f0%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #29303: non_atomic_requests decorator alters _non_atomic_requests attribute of original function

[Django]

#29303: non_atomic_requests decorator alters _non_atomic_requests attribute of
original function
-+-
 Reporter:  Alasdair Nicol   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Tim Graham):

 * stage:  Unreviewed => Accepted


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/071.8d819e26a7d28c0542d572d60b2d4464%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #29303: non_atomic_requests decorator alters _non_atomic_requests attribute of original function

[Django]

#29303: non_atomic_requests decorator alters _non_atomic_requests attribute of
original function
-+-
 Reporter:  Alasdair Nicol   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Old description:

> When calling `non_atomic_requests` with a function, it alters the
> `_non_atomic_requests` attribute of the original function.
>
> Here's an example:
>
> {{{
> from django.db import transaction
>
> @transaction.non_atomic_requests(using='default')
> def my_view(request):
> return HttpResponse('')
>
> assert my_view._non_atomic_requests == {'default'}  # passes
>
> wrapped_view = transaction.non_atomic_requests(using='other')
>
> assert wrapped_view._non_atomic_requests == {'default', 'other'}  #
> passes
> assert my_view._non_atomic_requests == {'default'}  # fails
> }}}
>
> I realise that this is a contrived example. It isn't an issue when
> `non_atomic_requests` is used as a decorator:
>
> {{{
> @transaction.non_atomic_requests(using='default')
> @transaction.non_atomic_requests(using='other')
> def my_view(request)
> return HttpResponse('')
> }}}

New description:

 When calling `non_atomic_requests` with a function, it alters the
 `_non_atomic_requests` attribute of the original function.

 Here's an example:

 {{{
 from django.test import TestCase

 # Create your tests here.

 from django.test import TestCase
 from django.db import transaction

 @transaction.non_atomic_requests(using='default')
 def my_view(request):
 return HttpResponse('')

 class TestViews(TestCase):

 def test(self):
 assert my_view._non_atomic_requests == {'default'}  # passes

 wrapped_view =
 transaction.non_atomic_requests(using='other')(my_view)

 assert wrapped_view._non_atomic_requests == {'default', 'other'}
 # passes
 assert my_view._non_atomic_requests == {'default'}  # fails
 }}}

 I realise that this is a contrived example. It isn't an issue when
 `non_atomic_requests` is used as a decorator:

 {{{
 @transaction.non_atomic_requests(using='default')
 @transaction.non_atomic_requests(using='other')
 def my_view(request)
 return HttpResponse('')
 }}}

--

Comment (by Alasdair Nicol):

 Sorry, there was a mistake in the `wrapped_view` line, I didn't apply it
 to the view. It should be:

 {{{
 wrapped_view = transaction.non_atomic_requests(using='other')(my_view)
 }}}

 I've updated the example in the ticket description.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/071.b1f30871a731bb70685d7828f420f6f4%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #29303: non_atomic_requests decorator alters _non_atomic_requests attribute of original function

[Django]

#29303: non_atomic_requests decorator alters _non_atomic_requests attribute of
original function
-+-
 Reporter:  Alasdair Nicol   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Tim Graham):

 I'm not able to reproduce. I created this test file:
 {{{ #!python
 from django.test import TestCase
 from django.db import transaction

 @transaction.non_atomic_requests(using='default')
 def my_view(request):
 return HttpResponse('')

 class TestViews(TestCase):

 def test(self):
 assert my_view._non_atomic_requests == {'default'}  # passes

 wrapped_view = transaction.non_atomic_requests(using='other')

 assert wrapped_view._non_atomic_requests == {'default', 'other'}
 # passes
 assert my_view._non_atomic_requests == {'default'}  # fails
 }}}
 I get:
 {{{
 Traceback (most recent call last):
   File "/home/tim/code/mysite/polls/test_29303.py", line 15, in test
 assert wrapped_view._non_atomic_requests == {'default', 'other'}  #
 passes
 AttributeError: 'function' object has no attribute '_non_atomic_requests'
 }}}
 and if I remove the assertion, the next assertion passes.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/071.e0e13ed79c04ebbbe34ae2077bef6823%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

[Django] #29303: non_atomic_requests decorator alters _non_atomic_requests attribute of original function

[Django]

#29303: non_atomic_requests decorator alters _non_atomic_requests attribute of
original function
-+-
   Reporter:  Alasdair   |  Owner:  nobody
  Nicol  |
   Type:  Bug| Status:  new
  Component:  Database   |Version:  master
  layer (models, ORM)|
   Severity:  Normal |   Keywords:
   Triage Stage: |  Has patch:  0
  Unreviewed |
Needs documentation:  0  |Needs tests:  0
Patch needs improvement:  0  |  Easy pickings:  0
  UI/UX:  0  |
-+-
 When calling `non_atomic_requests` with a function, it alters the
 `_non_atomic_requests` attribute of the original function.

 Here's an example:

 {{{
 from django.db import transaction

 @transaction.non_atomic_requests(using='default')
 def my_view(request):
 return HttpResponse('')

 assert my_view._non_atomic_requests == {'default'}  # passes

 wrapped_view = transaction.non_atomic_requests(using='other')

 assert wrapped_view._non_atomic_requests == {'default', 'other'}  # passes
 assert my_view._non_atomic_requests == {'default'}  # fails
 }}}

 I realise that this is a contrived example. It isn't an issue when
 `non_atomic_requests` is used as a decorator:

 {{{
 @transaction.non_atomic_requests(using='default')
 @transaction.non_atomic_requests(using='other')
 def my_view(request)
 return HttpResponse('')
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/056.1583a1f6825312d24bf28d51b9e0cf35%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.