Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecation Warning for **env_options in Jinja2Templates with Starlette >=0.28.0 #574

Closed
ptrstn opened this issue Aug 29, 2024 · 3 comments · Fixed by #575
Closed

Deprecation Warning for **env_options in Jinja2Templates with Starlette >=0.28.0 #574

ptrstn opened this issue Aug 29, 2024 · 3 comments · Fixed by #575
Labels
bug Something isn't working

Comments

@ptrstn
Copy link
Contributor

ptrstn commented Aug 29, 2024

I'm encountering a deprecation warning related to Jinja2Templates when running pytest tests with starlette>=0.28.0 :

.venv/lib/python3.11/site-packages/starlette/templating.py:106: DeprecationWarning: Extra environment options are deprecated. Use a preconfigured jinja2.Environment instead.
    warnings.warn(

This warning does not appear when installing starlette==0.27.0

According to the Starlette 0.28.0 release notes, the **env_options parameter in Jinja2Templates has been deprecated in favor of the new env parameter. The relevant pull request #2159 explains this change.

It seems likely that Starlette Admin (or other dependencies) are still using the deprecated **env_options, causing the warning when running tests with Starlette 0.28.0.

Steps to Reproduce:

  1. Project Structure:

    .
    ├── app.py
    ├── requirements-deprecation-warning.txt
    ├── requirements-no-warnings.txt
    └── test_app.py
    
  2. Source Code:

    app.py:

    from fastapi import FastAPI
    from starlette_admin.contrib.sqlmodel import Admin
    from sqlmodel import SQLModel, create_engine
    
    DATABASE_URL = "sqlite:///./test.db"
    engine = create_engine(DATABASE_URL, echo=True)
    SQLModel.metadata.create_all(engine)
    
    app = FastAPI()
    admin = Admin(engine, title="Simple Admin")
    admin.mount_to(app)

    test_app.py:

    import pytest
    from fastapi.testclient import TestClient
    from app import app
    
    @pytest.fixture(scope="module")
    def client():
        with TestClient(app) as client:
            yield client
    
    def test_admin_page(client):
        response = client.get("/admin")
        assert response.status_code == 200
        assert "Simple Admin" in response.text
  3. Requirements Files:

    requirements-deprecation-warning.txt:

    fastapi
    sqlmodel
    uvicorn
    starlette-admin
    jinja2
    sqlalchemy
    starlette==0.28.0
    pytest
    httpx==0.24.1
    

    requirements-no-warnings.txt:

    fastapi
    sqlmodel
    uvicorn
    starlette-admin
    jinja2
    sqlalchemy
    starlette==0.27.0
    pytest
    httpx==0.24.1
    
  4. Reproduce the Warning:

    pip install -r requirements-deprecation-warning.txt
    pytest

    Result: Deprecation Warning

    pip install -r requirements-no-warnings.txt
    pytest

    Result: All tests pass without warnings.

@ptrstn ptrstn added the bug Something isn't working label Aug 29, 2024
@ptrstn
Copy link
Contributor Author

ptrstn commented Aug 29, 2024

I think the issue lies in these lines:

def _setup_templates(self) -> None:
templates = Jinja2Templates(self.templates_dir, extensions=["jinja2.ext.i18n"])
templates.env.loader = ChoiceLoader(
[
FileSystemLoader(self.templates_dir),
PackageLoader("starlette_admin", "templates"),
]
)

The signature of Jinja2Templates looks like this:

class Jinja2Templates:
    @typing.overload
    def __init__(
        self,
        directory: str | PathLike[str] | typing.Sequence[str | PathLike[str]],
        *,
        context_processors: list[typing.Callable[[Request], dict[str, typing.Any]]]
        | None = None,
        **env_options: typing.Any,
    ) -> None: ...

So extensions=["jinja2.ext.i18n"] is passed indirectly through the **env_options argument of Jinja2Templates in Starlette.

@ptrstn
Copy link
Contributor Author

ptrstn commented Aug 29, 2024

Something like this should do the job:

from jinja2 import ChoiceLoader, Environment, FileSystemLoader, PackageLoader

def _setup_templates(self) -> None:
    env = Environment(
        loader=ChoiceLoader(
            [
                FileSystemLoader(self.templates_dir),
                PackageLoader("starlette_admin", "templates"),
            ]
        ), 
        extensions=["jinja2.ext.i18n"]
    )
    templates = Jinja2Templates(env=env)

But raises another Warning:

DeprecationWarning: The `name` is not the first parameter anymore. The first parameter should be the `Request` instance.
 Replace `TemplateResponse(name, {"request": request})` by `TemplateResponse(request, name)`.
   warnings.warn(

After further examination I saw that the parameters of TemplateResponse are swapped in _render_error:

async def _render_error(
self,
request: Request,
exc: Exception = HTTPException(status_code=HTTP_500_INTERNAL_SERVER_ERROR),
) -> Response:
assert isinstance(exc, HTTPException)
return self.templates.TemplateResponse(
"error.html",
{"request": request, "exc": exc},
status_code=exc.status_code,
)

and should rather look like this:

 async def _render_error( 
     self, 
     request: Request, 
     exc: Exception = HTTPException(status_code=HTTP_500_INTERNAL_SERVER_ERROR), 
 ) -> Response: 
     assert isinstance(exc, HTTPException) 
     return self.templates.TemplateResponse( 
         request=request,
         name="error.html", 
         context={"exc": exc}, 
         status_code=exc.status_code, 
     ) 

Also in all the other instantiations of TemplateResponse in base.py:

  • _render_list
  • _render_detail
  • _render_create (2x)
  • _render_edit (2x)

In auth.py:

  • render_login (3x)

And in views.py:

  • render

@jowilf
Copy link
Owner

jowilf commented Aug 30, 2024

Hi @ptrstn, thank you for reporting this. Please feel free to submit a PR to fix it.

ptrstn added a commit to ptrstn/starlette-admin that referenced this issue Aug 31, 2024
Addresses jowilf#574

DeprecationWarning: The `name` is not the first parameter anymore. The first parameter should be the `Request` instance.
  Replace `TemplateResponse(name, {"request": request})` by `TemplateResponse(request, name)`.
    warnings.warn(

Before Starlette 0.29.0, the name was the first parameter.

Also, before that, in previous versions, the request object was passed as part of the key-value pairs in the context for Jinja2.

See:
- https://www.starlette.io/release-notes/#0290
- encode/starlette#2191
- https://github.com/encode/starlette/blob/c78c9aac17a4d68e0647252310044502f1b7da71/starlette/templating.py#L166-L178
- https://fastapi.tiangolo.com/reference/templating/#fastapi.templating.Jinja2Templates.TemplateResponse
- https://fastapi.tiangolo.com/advanced/templates/#using-jinja2templates
ptrstn added a commit to ptrstn/starlette-admin that referenced this issue Aug 31, 2024
Resolves jowilf#574

According to the Starlette 0.28.0 release notes, the **env_options parameter in Jinja2Templates has been deprecated in favor of the new env parameter. The relevant pull request #2159 explains this change.

See:
- encode/starlette#2159
- https://www.starlette.io/release-notes/#0280
@jowilf jowilf closed this as completed in 9e50bc9 Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants