Ticket #28609 - Making REQUEST_URI available in runserver

[Jay Lynch]

Hi all!

Apologies if this I miss any conventions here, I've been working with 
Django for a long time now but this is my first message here. I'm here to 
chat about...


*(Closed) Ticket #28609  - 
Request URI*

I recently submitted a PR for a change which would make the original / raw 
URI of a request available to the development server's WSGI processing, as 
a parameter REQUEST_URI.

This would allow the creation of WSGI middleware which needs access to this 
information that can be used universally between both development with 
devserver and production with Nginx or similar.

Tim Graham suggested I post here if I disagree with it being flagged as 
wontfix on grounds of being something that would need to be discouraged as 
it enabled practices which are "not recommended".

I do disagree as there was a lack of any explanation or data around how 
this is a universal bad and whilst I agree it could be misused if a 
developer were naive of the details of how it can be used, I don't think 
that is sufficient justification. 


Lack of access to this means using very basic WSGI middleware with Django 
requires hugely disproportionate levels of effort. (eg. some suggestions 
offered were "write an alternative runserver" or "replace it with Apache", 
both likely measured in days of work for our team, in order to avoid a 
trivial code change which took 5 minutes and brings the dev server further 
in line with other commonly used WSGI servers)



*Reasoning*

I would like to enable access to this parameter so that we can continue to 
use runserver when working locally on the API for 
https://www.broadsheet.com.au - an Australian city guide and mid-sized 
publisher.

The specific use case we have for the raw URI is simple – there is a fairly 
universal form of search URL where spaces are replaced with + rather than 
URL encoded and in order to enable "+required" style binary operators, any 
literal + characters in the search query are URL encoded. (Attached is a 
screenshot of Google, Youtube, Bing, DuckDuckGo, Amazon all using this 
behaviour)

So a search for "test search +banana" (typically a query which requests 
results for "test search" but limits results to pages including the word 
"banana") is encoded as test+search+%2Bbanana

The benefits are clear: cleaner, human-friendly URLs, use of the same query 
format in places where spaces are used as delimiters, and maintaining use 
of + for binary modifiers, all with one syntax.

The WSGI middleware which performs this replacement is trivial, but 
impossible to use with runserver currently as it does not provide the 
information.



*Alternatives*

It is not possible to implement this as a library or add-on as the dev 
server does not pass this data downstream. Replacing the development server 
is a high cost if it is for this reason alone.

Alternative implementations within dev server could include full support 
for WSGI parameters, which seems excessive, or custom query string 
processing for this use case, which would be less compatible and may break 
other expected behaviours.



*Potential Downsides*

*Could be used inappropriately and may need to be cautioned against*

I don't see why there is any need for it to be documented, it is providing 
optional access to an internal, as is the case with WSGI parameters in 
other web servers. That aside, I disagree that requiring caution 
invalidates the value in making a tool available to users. 


*REQUEST_URI is not actually a standard *

This is true but it has become a de facto standard which is supported by 
most major web servers and in many cases enabled by default. Enabling it 
allows consistent WSGI middleware use.


*Cluttering the WSGI environment*

Given a very similar mechanism is being used to pass information like color 
highlighting status around it seems inconsistent to call this a concern.


*The implementation is flawed*

I would be very happy to receive feedback here, this is my first time 
submitting a patch / PR.

 

*Code*

See https://github.com/django/django/pull/9101 – the change involved is 
minimal, it simply has the dev server add the raw URI as a parameter in the 
WSGI environment so that middleware can use it:


-return super(WSGIRequestHandler, self).get_environ()

+environ = super(WSGIRequestHandler, self).get_environ()
+environ[str("REQUEST_URI")] = self.path
+return environ


We have been using a branch with this change applied to 1.11 in both 
development and production since the PR was submitted, though the related 
WSGI middleware is not yet in production.



Thanks for your time!


Jay Lynch




-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe fro

Re: about ticket 28588- has_perm hide non-existent permissions

[Tim Graham]

I suppose we can tentatively accept the ticket, but I looked at the code 
briefly and agree with Florian's assessment. If someone proposes a patch, 
we can evaluate it, however, I don't see a simple way forward that wouldn't 
have a security risk or an adverse effect on performance. Given the 
philosophy, "complexity is the enemy of security," I'd lean toward keeping 
the permissions checking code simple instead of adding some other logic 
based on DEBUG.

On Wednesday, September 27, 2017 at 9:48:24 AM UTC-4, Florian Apolloner 
wrote:
>
> I do not think it would be feasible to check existing permissions. For 
> one, not every backend uses the Permission class Django supplies and 
> get_all_permissions 
> can cause performance issues so it should be used sparingly.
>
> Cheers,
> Florian
>
> On Sunday, September 24, 2017 at 4:56:40 PM UTC+2, moshe nahmias wrote:
>>
>> Hi,
>> I am a python developer and like to use Django for web development.
>> Since I like the framework I want to contribute back, so I looked at the 
>> open tickets to find something I can start with contributing and found 
>> ticket 28588.
>>
>> This ticket is about when checking if the user has permission for some 
>> action if the user is super user he/she gets it all the time, even when the 
>> permission doesn't exist, and this is not developer friendly because the 
>> developer can mistakenly think that everything is fine even when the 
>> permission doesn't exist.
>>
>> As I understand (and correct me if I'm wrong) there should be a 
>> discussion about if we want to do this.
>>
>> If accepted I would like to do this, I think it's an easy enough change 
>> for a new contributor like me.
>>
>> As I understand the ticket the problem is that a developer gets confused 
>> on this behaviour (and it's illogical) that the super user is having a 
>> permission that doesn't exist.
>>
>> What do you think? (I think I will discuss my solution or optional 
>> solutions after we decide if we want to change this behaviour)
>>
>> [1] https://code.djangoproject.com/ticket/28588
>>
>
>

-- 
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/7e34b477-46ff-4f48-a45a-e4d0f8132f54%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: about ticket 28588- has_perm hide non-existent permissions

[Shai Berger]

Can we define a new API on the permission backend, "verify_permission_exists()" 
or some such, and just call it if settings.DEBUG and it is provided? That 
doesn't seem very complex to me, and doesn't necessarily imply a huge 
performance hit (even in DEBUG).

On Thursday 28 September 2017 15:50:04 Tim Graham wrote:
> I suppose we can tentatively accept the ticket, but I looked at the code
> briefly and agree with Florian's assessment. If someone proposes a patch,
> we can evaluate it, however, I don't see a simple way forward that wouldn't
> have a security risk or an adverse effect on performance. Given the
> philosophy, "complexity is the enemy of security," I'd lean toward keeping
> the permissions checking code simple instead of adding some other logic
> based on DEBUG.
> 
> On Wednesday, September 27, 2017 at 9:48:24 AM UTC-4, Florian Apolloner
> 
> wrote:
> > I do not think it would be feasible to check existing permissions. For
> > one, not every backend uses the Permission class Django supplies and
> > get_all_permissions can cause performance issues so it should be used
> > sparingly.
> > 
> > Cheers,
> > Florian
> > 
> > On Sunday, September 24, 2017 at 4:56:40 PM UTC+2, moshe nahmias wrote:
> >> Hi,
> >> I am a python developer and like to use Django for web development.
> >> Since I like the framework I want to contribute back, so I looked at the
> >> open tickets to find something I can start with contributing and found
> >> ticket 28588.
> >> 
> >> This ticket is about when checking if the user has permission for some
> >> action if the user is super user he/she gets it all the time, even when
> >> the permission doesn't exist, and this is not developer friendly
> >> because the developer can mistakenly think that everything is fine even
> >> when the permission doesn't exist.
> >> 
> >> As I understand (and correct me if I'm wrong) there should be a
> >> discussion about if we want to do this.
> >> 
> >> If accepted I would like to do this, I think it's an easy enough change
> >> for a new contributor like me.
> >> 
> >> As I understand the ticket the problem is that a developer gets confused
> >> on this behaviour (and it's illogical) that the super user is having a
> >> permission that doesn't exist.
> >> 
> >> What do you think? (I think I will discuss my solution or optional
> >> solutions after we decide if we want to change this behaviour)
> >> 
> >> [1] https://code.djangoproject.com/ticket/28588

bulk_create on Postgresql: on conflict do nothing / post_save signal

[Дилян Палаузов]

Hello,

I want after a user request to be sure that certain objects are stored 
in a Postgres database, even if before the request some of the objects 
were there.


The only way I can do this with django, not talking about raw sql, is 
with "for obj in objects: Model.objects.get_or_create(obj)".  It works, 
but creates several INSERTs, and is hence suboptimal.


I cannot use bulk_create(), which squeezes all the INSERTs to a single 
one, as it does not work, if any of the to-be-inserted rows was already 
in the database.


In Postgresql this can be achieved by sending "INSERT ... ON CONFLICT DO 
NOTHING".


I propose changing the interface of QuerySet.bulk_create to accept one 
more parameter on_conflict, that can be a string e.g. "DO NOTHING" or 
Q-objects (which could be used to implement ON CONFLICT DO UPDATE SET 
... WHERE ...


def bulk_create(self, objs, batch_size=None, on_conflict=None): ...

What are the arguments against or in favour?

The further, bulk_create() does not send post_save signal, because it is 
difficult to implement with the standard backends, except with postgresql.


I propose extending the implementation to send the signal:

https://code.djangoproject.com/ticket/28641#comment:1

when Postgresql is used.  I assume there a no users, who want to get a 
(post_save) signal on save() but not on bulk_create().


Combining ON CONFLICT DO NOTHING with sending post_save gets however 
nasty, as "INSERT ... ON CONFLICT DO NOTHING RETURNING id;" does not 
return anything on unchanged rows, hence the system knows at the end how 
much rows were changed, but not which, so it cannot determine for which 
objects to send post_save.  At least I have not found a way how to 
figure out which rows were inserted/not inserted.


However, this can be achieved by RETURNING * and then comparing the 
returned objects to the sent objects, eventually making bulk_create() 
return the objects actually inserted in the database.


These changes will allow a switch to a single INSERT on Postgresql.

Regards
  Дилян

--
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/daa88462-c095-dfcc-2ce7-6d34f6bbc2f6%40aegee.org.
For more options, visit https://groups.google.com/d/optout.

Re: bulk_create on Postgresql: on conflict do nothing / post_save signal

[Tom Forbes]

I've been in similar situations before, you can usually get away with using
a single query to fetch existing records and only pass data that doesn't
exist to bulk_create. This works great for a single identity column, but if
you have multiple it gets messy.

It seems all supported databases offer at least ON CONFLICT IGNORE in some
form or another, with pretty similar syntax.

On 28 Sep 2017 18:11, "Дилян Палаузов"  wrote:

>
> Hello,
>
> I want after a user request to be sure that certain objects are stored in
> a Postgres database, even if before the request some of the objects were
> there.
>
> The only way I can do this with django, not talking about raw sql, is with
> "for obj in objects: Model.objects.get_or_create(obj)".  It works, but
> creates several INSERTs, and is hence suboptimal.
>
> I cannot use bulk_create(), which squeezes all the INSERTs to a single
> one, as it does not work, if any of the to-be-inserted rows was already in
> the database.
>
> In Postgresql this can be achieved by sending "INSERT ... ON CONFLICT DO
> NOTHING".
>
> I propose changing the interface of QuerySet.bulk_create to accept one
> more parameter on_conflict, that can be a string e.g. "DO NOTHING" or
> Q-objects (which could be used to implement ON CONFLICT DO UPDATE SET ...
> WHERE ...
>
> def bulk_create(self, objs, batch_size=None, on_conflict=None): ...
>
> What are the arguments against or in favour?
>
> The further, bulk_create() does not send post_save signal, because it is
> difficult to implement with the standard backends, except with postgresql.
>
> I propose extending the implementation to send the signal:
>
> https://code.djangoproject.com/ticket/28641#comment:1
>
> when Postgresql is used.  I assume there a no users, who want to get a
> (post_save) signal on save() but not on bulk_create().
>
> Combining ON CONFLICT DO NOTHING with sending post_save gets however
> nasty, as "INSERT ... ON CONFLICT DO NOTHING RETURNING id;" does not return
> anything on unchanged rows, hence the system knows at the end how much rows
> were changed, but not which, so it cannot determine for which objects to
> send post_save.  At least I have not found a way how to figure out which
> rows were inserted/not inserted.
>
> However, this can be achieved by RETURNING * and then comparing the
> returned objects to the sent objects, eventually making bulk_create()
> return the objects actually inserted in the database.
>
> These changes will allow a switch to a single INSERT on Postgresql.
>
> Regards
>   Дилян
>
> --
> 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/ms
> gid/django-developers/daa88462-c095-dfcc-2ce7-6d34f6bbc2f6%40aegee.org.
> 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/CAFNZOJOUGzYixc4cvPF2%2B_VTo2YwqwVDk%2BTkErxW3hjXvpaXbQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.