Skip to content

Set request in template context#1400

Closed
aminalaee wants to merge 7 commits intomasterfrom
set-request-context-in-templating
Closed

Set request in template context#1400
aminalaee wants to merge 7 commits intomasterfrom
set-request-context-in-templating

Conversation

@aminalaee
Copy link
Contributor

@aminalaee aminalaee commented Jan 8, 2022

Closes #482 according to this.

Sets up request for template context automatically. So instead of this:

async def homepage(request):
    return templates.TemplateResponse('index.html', {'request': request})

We can do:

async def homepage(request):
    return templates.TemplateResponse('index.html')

So the context argument to templates.TemplateResponse is not required anymore.

@aminalaee aminalaee changed the title Set request in templating context automatically Set request in templating context automatically Jan 8, 2022
@aminalaee aminalaee requested a review from a team January 8, 2022 20:23
@aminalaee aminalaee mentioned this pull request Jan 9, 2022
8 tasks
@aminalaee aminalaee changed the title Set request in templating context automatically Set request in template context Jan 9, 2022
@lovelydinosaur
Copy link
Contributor

Okay, I've been looking through this for review.
I hadn't appreciated a couple of things until just now...

  • I think this change will end up setting Content-Length: 0 on template responses. (Which would be broken.)
  • Switching TemplateResponse so that it renders at the point it is called, rather than on __init__ is a bit of a behaviour changing thing for use to do. If response.body is inspected directly in the endpoint, it'll be b"".

I'm not sure what the upshot of those ought to be for us. One option is that we just decide this wasn't the right thing for us to do, and drop those issues. Another would be to make sure we set the Content-Length header properly, and then just be okay with the other behaviour changes.

?

@aminalaee
Copy link
Contributor Author

aminalaee commented Jan 10, 2022

I have to admit I didn't know about the first issue. I thought we just had to live with the second one.

I think unless we find a clean way to achieve the desired outcome, if possible avoid request in context but return proper response and have stable behaviour for Response, we should drop this and leave it as is.

Update: I'll spend more time on it and see if that's possible.

@lovelydinosaur
Copy link
Contributor

I guess let's just drop this for now, and close the related issues as "we've decided not to do this". (?)
It'd be very nice if it was less awkward. But it's not, so.

@aminalaee aminalaee closed this Jan 11, 2022
@aminalaee aminalaee deleted the set-request-context-in-templating branch February 10, 2022 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rendering templates without explicitly passing request

2 participants