Re: Possible Bug in RegexURLResolver

[Cristiano Coelho]

Sorry for my above answer I think the actual locks should go like this, so 
all threads can actually go to the wait line while _populating is True.

def _populate(self):

self.lock.acquire()

if self._populating:
self.lock.wait()
self.lock.release()
return

self._populating = True

self.lock.release()

...

self.lock.acquire()
self._populating = False
self.lock.notify_all()
self.lock.release()



El lunes, 11 de julio de 2016, 22:14:04 (UTC-3), Cristiano Coelho escribió:
>
> Wouldn't a standard Lock do the trick? Also you are still vulnerable to a 
> race condition when reading self._populating, if the goal is to avoid 
> populating the object more than once in a short interval (in a case where 
> multiple requests hit the server before the object is initialized for the 
> first time?) you are still running all the critical code on all threads if 
> they check self.populating before it is set to True.
>
> Would a condition work better in this case? Something like this.. (add any 
> missing try/finally/with that might be required), warning, NOT TESTED, just 
> an idea.
> The below code will not be prone to race conditions as the variables are 
> always read/set under a lock, and also the critical section code will be 
> only executed ONCE even if multiple threads attempt to run it at once, 
> while still locking all threads to prevent returning before the code is 
> done.
>
> def __init__(self, regex, urlconf_name, default_kwargs=None, app_name=None
> , namespace=None):
> 
> ...
>
> self._populating = False
> self.lock = threading.Condition()
>
> def _populate(self):
>
> self.lock.acquire()
> 
> if self._populating:
> self.lock.wait()
> self.lock.release()
> return
> 
> self._populating = True
>
> ...
>
> self._populating = False
> self.lock.notify_all()
> self.lock.release()
>
>
>
>
>
> El lunes, 11 de julio de 2016, 10:08:50 (UTC-3), a.br...@rataran.com 
> escribió:
>>
>> Hello everyone.
>>
>> As suggested by Markus on django-users group, I'm posting this here too.
>>
>> ---
>>
>> I'm using django (1, 10, 0, u'beta', 1).
>>
>> When I try to reverse url in shell everything goes fine.
>>
>> When under nginx/uwsgi with many concurrent request I get 
>>
>> ... /local/lib/python2.7/site-packages/django/urls/resolvers.py", line 
>> 241, in reverse_dict
>> return self._reverse_dict[language_code]
>> KeyError: 'it'
>>
>> After a wile I figured out that RegexURLResolver is memoized by 
>> get_resolver and so it acts like a singleton for a certain number of 
>> requests.
>>
>> Analyzing the code of  RegexURLResolver I found that the method 
>> _poupulate will return directly if it has been called before and not yet 
>> finished.
>>
>> ...
>> def _populate(self):
>> if self._populating:
>> return
>> self._populating = True
>> ...  
>>
>> if used for recursive call in a single thread this will not hurt, but in 
>> my case in uwsgi multi thread mode I got the error.
>>
>> here is my quick and dirty fix:
>>
>> class RegexURLResolver(LocaleRegexProvider):
>> def __init__(self, regex, urlconf_name, default_kwargs=None, 
>> app_name=None, namespace=None):
>> 
>> ...
>>
>> self._populating = False
>> self.RLock = threading.RLock()
>>
>> ...
>>
>>def _populate(self):
>> if self._populating:
>> self.RLock.acquire()
>> self.RLock.release()
>> return
>> self._populating = True
>> self.RLock.acquire()
>> 
>> ...
>>
>> self._populating = False
>> self.RLock.release()
>>
>>
>> Does anyone know if there is a better solution?
>>
>> Thank you.
>>
>

-- 
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/922ca727-dd60-41dd-9b78-b8d8fe9ebd16%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Possible Bug in RegexURLResolver

[Cristiano Coelho]

Wouldn't a standard Lock do the trick? Also you are still vulnerable to a 
race condition when reading self._populating, if the goal is to avoid 
populating the object more than once in a short interval (in a case where 
multiple requests hit the server before the object is initialized for the 
first time?) you are still running all the critical code on all threads if 
they check self.populating before it is set to True.

Would a condition work better in this case? Something like this.. (add any 
missing try/finally/with that might be required), warning, NOT TESTED, just 
an idea.
The below code will not be prone to race conditions as the variables are 
always read/set under a lock, and also the critical section code will be 
only executed ONCE even if multiple threads attempt to run it at once, 
while still locking all threads to prevent returning before the code is 
done.

def __init__(self, regex, urlconf_name, default_kwargs=None, app_name=None, 
namespace=None):

...

self._populating = False
self.lock = threading.Condition()

def _populate(self):

self.lock.acquire()

if self._populating:
self.lock.wait()
self.lock.release()
return

self._populating = True

...

self._populating = False
self.lock.notify_all()
self.lock.release()





El lunes, 11 de julio de 2016, 10:08:50 (UTC-3), a.br...@rataran.com 
escribió:
>
> Hello everyone.
>
> As suggested by Markus on django-users group, I'm posting this here too.
>
> ---
>
> I'm using django (1, 10, 0, u'beta', 1).
>
> When I try to reverse url in shell everything goes fine.
>
> When under nginx/uwsgi with many concurrent request I get 
>
> ... /local/lib/python2.7/site-packages/django/urls/resolvers.py", line 
> 241, in reverse_dict
> return self._reverse_dict[language_code]
> KeyError: 'it'
>
> After a wile I figured out that RegexURLResolver is memoized by 
> get_resolver and so it acts like a singleton for a certain number of 
> requests.
>
> Analyzing the code of  RegexURLResolver I found that the method _poupulate 
> will return directly if it has been called before and not yet finished.
>
> ...
> def _populate(self):
> if self._populating:
> return
> self._populating = True
> ...  
>
> if used for recursive call in a single thread this will not hurt, but in 
> my case in uwsgi multi thread mode I got the error.
>
> here is my quick and dirty fix:
>
> class RegexURLResolver(LocaleRegexProvider):
> def __init__(self, regex, urlconf_name, default_kwargs=None, 
> app_name=None, namespace=None):
> 
> ...
>
> self._populating = False
> self.RLock = threading.RLock()
>
> ...
>
>def _populate(self):
> if self._populating:
> self.RLock.acquire()
> self.RLock.release()
> return
> self._populating = True
> self.RLock.acquire()
> 
> ...
>
> self._populating = False
> self.RLock.release()
>
>
> Does anyone know if there is a better solution?
>
> Thank you.
>

-- 
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/0782172d-2648-4770-83ff-93dfd00ade17%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Exceptions caught by the "include" template tag make it hard to rely on tests

[Tim Graham]

Has anyone relied on the exception silencing behavior as a "feature" when 
using {% include %}? It doesn't seem that's really possible given you can't 
use that feature when template.debug=True. Thus, the current behavior 
doesn't strike me as a reasonable default. Perhaps we can change it after a 
deprecation. Does anyone really want an alternative to keep the silencing 
behavior?

>From a user's perspective, I think it's more confusing to see a partial and 
possibly broken page (but not necessarily knowing that it's broken) than an 
error page.

On Monday, July 11, 2016 at 6:17:58 PM UTC-4, Shai Berger wrote:
>
> On Monday 11 July 2016 22:45:31 Florian Apolloner wrote: 
> > On Monday, July 11, 2016 at 7:56:34 PM UTC+2, Tim Graham wrote: 
> > > As for me as a developer, I'd support removing the exception silencing 
> > > that {% include %} does so that errors appear alongside any other 
> > > exceptions (e.g. in Sentry) rather than in the django.template logger 
> > > (that logging was added in Django 1.9). 
> > 
> > Dito, most tags should error out where it makes sense, but not sure if 
> we 
> > can easily change that :( 
>
> It must be opt-in; now that we have template engines and they're 
> configured in 
> dictionaries, we have plenty of room for that, though. 
>
> Another option to consider is make them error out only under the 
> test-client. 
>
> My 2 cents, 
> Shai. 
>

-- 
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/4c2355dc-9ddd-4bad-9677-78cb181f1a32%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Admin-Actions also in the object details view

[Cristiano Coelho]

Thanks, I guess that can do it for now. It's a shame the one who started 
implementing this in django itself just abandoned it :(

El lunes, 11 de julio de 2016, 8:55:17 (UTC-3), Alex Riina escribió:
>
> Here's an implementation:
>
> https://github.com/crccheck/django-object-actions
>
> Combining with other admin plugins that change the template requires 
> overriding the template to fit both changes in.
>
> The actions are GETs and have no card protection.
>
>

-- 
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/fbd1acfe-3a68-4b6e-90b2-bf8a42f0e2a9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Exceptions caught by the "include" template tag make it hard to rely on tests

[Shai Berger]

On Monday 11 July 2016 22:45:31 Florian Apolloner wrote:
> On Monday, July 11, 2016 at 7:56:34 PM UTC+2, Tim Graham wrote:
> > As for me as a developer, I'd support removing the exception silencing
> > that {% include %} does so that errors appear alongside any other
> > exceptions (e.g. in Sentry) rather than in the django.template logger
> > (that logging was added in Django 1.9).
> 
> Dito, most tags should error out where it makes sense, but not sure if we
> can easily change that :(

It must be opt-in; now that we have template engines and they're configured in 
dictionaries, we have plenty of room for that, though.

Another option to consider is make them error out only under the test-client.

My 2 cents,
Shai.

Re: Possible Bug in RegexURLResolver

[Markus Holtermann]

Hey,

thanks for posting this here. I opened
https://code.djangoproject.com/ticket/26888 to keep track of this. Can I
get somebody with threading experience in Python tell me if your
proposed patch makes sense?

Also, I marked this as a release blocker for 1.10 as I introduced this
during a patch for another issue
https://code.djangoproject.com/ticket/24931

/Markus

On Sun, Jul 10, 2016 at 11:47:35PM -0700, a.bra...@rataran.com wrote:

Hello everyone.

As suggested by Markus on django-users group, I'm posting this here too.

---

I'm using django (1, 10, 0, u'beta', 1).

When I try to reverse url in shell everything goes fine.

When under nginx/uwsgi with many concurrent request I get

... /local/lib/python2.7/site-packages/django/urls/resolvers.py", line 241,
in reverse_dict
   return self._reverse_dict[language_code]
KeyError: 'it'

After a wile I figured out that RegexURLResolver is memoized by
get_resolver and so it acts like a singleton for a certain number of
requests.

Analyzing the code of  RegexURLResolver I found that the method _poupulate
will return directly if it has been called before and not yet finished.

   ...
   def _populate(self):
   if self._populating:
   return
   self._populating = True
   ...

if used for recursive call in a single thread this will not hurt, but in my
case in uwsgi multi thread mode I got the error.

here is my quick and dirty fix:

class RegexURLResolver(LocaleRegexProvider):
   def __init__(self, regex, urlconf_name, default_kwargs=None,
app_name=None, namespace=None):

   ...

   self._populating = False
   self.RLock = threading.RLock()

   ...

  def _populate(self):
   if self._populating:
   self.RLock.acquire()
   self.RLock.release()
   return
   self._populating = True
   self.RLock.acquire()

   ...

   self._populating = False
   self.RLock.release()


Does anyone know if there is a better solution?

Thank you.

--
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/308c7d89-27e5-4841-85b1-55b0584cef3b%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/20160711204150.GE11144%40inel.local.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: PGP signature

Re: Exceptions caught by the "include" template tag make it hard to rely on tests

[Florian Apolloner]

On Monday, July 11, 2016 at 7:56:34 PM UTC+2, Tim Graham wrote:
>
> As for me as a developer, I'd support removing the exception silencing 
> that {% include %} does so that errors appear alongside any other 
> exceptions (e.g. in Sentry) rather than in the django.template logger (that 
> logging was added in Django 1.9).
>

Dito, most tags should error out where it makes sense, but not sure if we 
can easily change that :( 

-- 
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/9aad091e-63ad-42dd-bc67-87bb3ef91e5f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Exceptions caught by the "include" template tag make it hard to rely on tests

[Tim Graham]

Found in another thread [0]: "Some time in late 2003, Adrian and I decided 
that errors in templates  were best handled silently - the idea was that it 
would prevent untrained template editors from being able to 500-error a 
site with a typo."

And another [1]: "The broad reasoning is that a partial page rendering is 
preferable to a 500 error when rendering a template. This is driven by 
production requirements -- the end user shouldn't ever see a 500 error."

As for me as a developer, I'd support removing the exception silencing that 
{% include %} does so that errors appear alongside any other exceptions 
(e.g. in Sentry) rather than in the django.template logger (that logging 
was added in Django 1.9).

[0] 
https://groups.google.com/d/msg/django-developers/8yMzgEWsLl4/H7ITMVMkZcwJ
[1] 
https://groups.google.com/d/msg/django-developers/KpDS2tWLp7U/y7zli4P79nkJ

On Monday, July 11, 2016 at 4:06:11 AM UTC-4, Sylvain Fankhauser wrote:
>
> Hi,
>
> I'm bringing this back to the table since I can't believe I'm the only one 
> to find this behaviour weird and possibly dangerous. I'd really be 
> interested in getting your opinion on this!
>
> Thanks,
> Sylvain
>
> 2016-06-09 14:37 GMT+02:00 Sylvain Fankhauser  >:
>
>> Hi all,
>>
>> I got really frustrated today trying to understand why a bug could get 
>> past my test suite but fail in my dev environment. Finally Baptiste 
>> Mispelon pointed me to the include template tag documentation 
>> :
>>
>> If the included template causes an exception while it’s rendered 
>>> (including if it’s missing or has syntax errors), the behavior varies 
>>> depending on the template engine's debug option (if not set, this option 
>>> defaults to the value of DEBUG). When debug mode is turned on, an exception 
>>> like TemplateDoesNotExist or TemplateSyntaxError will be raised. When debug 
>>> mode is turned off, {% include %} logs a warning to the django.template 
>>> logger with the exception that happens while rendering the included 
>>> template and returns an empty string.
>>>
>>
>> That's a real problem when running tests, because since the exception is 
>> just silenced your tests will still pass although there was actually some 
>> error in your code. The current way to deal with this is, I guess, to set 
>> the template engine DEBUG option to True in the test environment but that 
>> doesn't seem right to me.
>>
>> I don't have a solution right now and I know this would be a big 
>> backwards incompatible change and the chances for this to change are 
>> abysmally low. But maybe we could just add an option for that (to make 
>> exceptions pop out of the include tag), set its default value to match the 
>> current behaviour, and change it with a clear deprecation plan?
>>
>> Cheers,
>> Sylvain
>>
>
>

-- 
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/d8452749-e30f-4a91-af32-6cb862cb431d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Possible Bug in RegexURLResolver

[a . branco]

Hello everyone.

As suggested by Markus on django-users group, I'm posting this here too.

---

I'm using django (1, 10, 0, u'beta', 1).

When I try to reverse url in shell everything goes fine.

When under nginx/uwsgi with many concurrent request I get 

... /local/lib/python2.7/site-packages/django/urls/resolvers.py", line 241, 
in reverse_dict
return self._reverse_dict[language_code]
KeyError: 'it'

After a wile I figured out that RegexURLResolver is memoized by 
get_resolver and so it acts like a singleton for a certain number of 
requests.

Analyzing the code of  RegexURLResolver I found that the method _poupulate 
will return directly if it has been called before and not yet finished.

...
def _populate(self):
if self._populating:
return
self._populating = True
...  

if used for recursive call in a single thread this will not hurt, but in my 
case in uwsgi multi thread mode I got the error.

here is my quick and dirty fix:

class RegexURLResolver(LocaleRegexProvider):
def __init__(self, regex, urlconf_name, default_kwargs=None, 
app_name=None, namespace=None):

...

self._populating = False
self.RLock = threading.RLock()

...

   def _populate(self):
if self._populating:
self.RLock.acquire()
self.RLock.release()
return
self._populating = True
self.RLock.acquire()

...

self._populating = False
self.RLock.release()


Does anyone know if there is a better solution?

Thank you.

-- 
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/308c7d89-27e5-4841-85b1-55b0584cef3b%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Admin-Actions also in the object details view

[Alex Riina]

s/card/csrf/

autocorrect grabbed that one from me.

On Monday, July 11, 2016 at 7:55:17 AM UTC-4, Alex Riina wrote:
>
> Here's an implementation:
>
> https://github.com/crccheck/django-object-actions
>
> Combining with other admin plugins that change the template requires 
> overriding the template to fit both changes in.
>
> The actions are GETs and have no card protection.
>
>

-- 
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/0b29d665-bbd3-4470-9b4d-120b6d88bae3%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Admin-Actions also in the object details view

[Alex Riina]

Here's an implementation:

https://github.com/crccheck/django-object-actions

Combining with other admin plugins that change the template requires overriding 
the template to fit both changes in.

The actions are GETs and have no card protection.

-- 
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/ff365877-e4c8-45a7-ab08-9456982c3b89%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: Exceptions caught by the "include" template tag make it hard to rely on tests

[Sylvain Fankhauser]

Hi,

I'm bringing this back to the table since I can't believe I'm the only one
to find this behaviour weird and possibly dangerous. I'd really be
interested in getting your opinion on this!

Thanks,
Sylvain

2016-06-09 14:37 GMT+02:00 Sylvain Fankhauser 
:

> Hi all,
>
> I got really frustrated today trying to understand why a bug could get
> past my test suite but fail in my dev environment. Finally Baptiste
> Mispelon pointed me to the include template tag documentation
> :
>
> If the included template causes an exception while it’s rendered
>> (including if it’s missing or has syntax errors), the behavior varies
>> depending on the template engine's debug option (if not set, this option
>> defaults to the value of DEBUG). When debug mode is turned on, an exception
>> like TemplateDoesNotExist or TemplateSyntaxError will be raised. When debug
>> mode is turned off, {% include %} logs a warning to the django.template
>> logger with the exception that happens while rendering the included
>> template and returns an empty string.
>>
>
> That's a real problem when running tests, because since the exception is
> just silenced your tests will still pass although there was actually some
> error in your code. The current way to deal with this is, I guess, to set
> the template engine DEBUG option to True in the test environment but that
> doesn't seem right to me.
>
> I don't have a solution right now and I know this would be a big backwards
> incompatible change and the chances for this to change are abysmally low.
> But maybe we could just add an option for that (to make exceptions pop out
> of the include tag), set its default value to match the current behaviour,
> and change it with a clear deprecation plan?
>
> Cheers,
> Sylvain
>

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