Working Jinja2 url_for with test #1004
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
for more information, see https://pre-commit.ci
CodSpeed Performance ReportMerging #1004 will not alter performanceComparing Summary
|
|
I'm stuck. See #996 (comment) |
for more information, see https://pre-commit.ci
dave42w
changed the title
dave42w
changed the title
…n. ruff fixes. Safety check for no robyn object.
|
Hi @sansyrox I've just added the support for URL parameters. All committed with tests. So from my previous comment today b) is done. Feedback on a) c) and d) appreciated. Thanks Dave |
|
Hi @sansyrox I've sorted out the pyproject.toml problem (a) :-) |
Hi, Wondering if you are able to do that review or if there is more I need to do. Thanks |
|
Hey @dave42w 👋 Apologies for the super late review. My work has been mental. I will try to complete my review by tonight or tomorrow for sure 😄 |
| @abstractmethod | ||
| def get_function_url(self, function_name: str, route_type: str = "GET", **kwargs) -> str: ... |
There was a problem hiding this comment.
@dave42w , should it be a part of the ABC ?
There was a problem hiding this comment.
I put it in the ABC because I assume that if you switched from Jinja2 to a different templating engine you would still want to be able to call this function so you can have less brittle templates (ones that change if you change a subrouter prefix or the route for a function.
| def set_robyn(self, robyn: Robyn) -> None: | ||
| """ | ||
| The get_function_url needs to have access to the list of routes stored in the apps Robyn object | ||
|
|
||
| Args: | ||
| robyn (Robyn): The top instance of the Robyn class for this app. | ||
| """ |
There was a problem hiding this comment.
I am sorry if I am being a bit daft but why is this function needed?
There was a problem hiding this comment.
When we are rendering a template that includes a call into the TemplateInterface get_function_url(...) that function needs to get the live and complete list of routes. That means we can't have the TemplateInterface keep a copy of the routes that existed when it was created. Instead, when the call is made we check the live list of routes. The only place I could see that we can get that live list of routes from is the main app Robyn instance.
I couldn't see a way to get hold of the Robyn instance automagically. So it was either a set method or the constructor.
I decided to go for a set method as the constructor for JiujaTemplate already has 3 arguments plus self (I figured it will impact the existing code less and be easier to add additions templating packages)
If projects don't use get_function_url(...) then they don't need to call set_robyn.
Long term I want to find a way to get to the routes via a router singleton and to have the router do the work to return the URL. But I don't think we have that possibility yet. And it won't change the templates (only thing would be that set_robyn would not be needed but we can make it do nothing and depreciate it over a couple of releases)
|
@sansyrox |
|
Hi @sansyrox |
|
Hi @sansyrox I wanted to check if you are happy for the conversations to be marked as resolved? |
|
Hi, @sansyrox I think the test failures are CI timeouts, this has passed all the tests at some point since the last change. Can we merge? |
Description
This PR starts the process of implementing url_for so that Jinja templates can reference Robyn endpoints by function name rather than path see #996
Summary
[Updated]
This PR adds a url_for that returns a fixed string and a test for this.3rd Nov 24 Now looks up functions via the Robyn object router
7th Nov. All finished.
Changes are limited to 7 files (5 of them related to testing)
add_function_to_globals())sync_authfunction/sync_authin an href)TODO handle positional and named argumentsTODO add set_robyn and url_for to template_interfaceTODO warn if no Robyn object set using set_robynTODO improve response to an invalid route
TODO rename from url_for to get_function_url in preparation for adding get_static_urlTODO remove@pytest.mark.url_forfrom test_get_requests.py`jinja_template = JinjaTemplate(".", "utf-8")#solves my git locale pre-commit issueTODO add unit testsTODO add unit tests that cover combinations of url'sTODO documentation
PR Checklist
Please ensure that:
Pre-Commit Instructions:
I think I've done that. I'm running the tests from a separate clone which I have had to modify to work with Python 3.12.7