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

docs: SQLAlchemyInitPlugin to SQLAlchemyPlugin fixes SerializationException for docs #3475

Merged
merged 19 commits into from May 17, 2024

Conversation

JorenSix
Copy link
Contributor

@JorenSix JorenSix commented May 6, 2024

Description

This fixes a SerializationException when running the demo with dummy data included. The example now is fixed by using the SQLAlchemyPlugin in stead of the SQLAlchemyInitPlugin and dummy data is added which immediately shows the expected behaviour: a JSON list with authors.

See issue #3464

Closes

Closes #3464

Now uses the SQLAlchemyPlugin in stead of SQLAlchemyInitPlugin, see litestar-org#3464
Update sqlalchemy_declarative_models.py
@JorenSix JorenSix requested review from a team as code owners May 6, 2024 08:16
@github-actions github-actions bot added area/docs This PR involves changes to the documentation size: small pr/external Triage Required 🏥 This requires triage labels May 6, 2024
@JorenSix JorenSix changed the title SQLAlchemyInitPlugin to SQLAlchemyPlugin fixes SerializationException for docs docs: SQLAlchemyInitPlugin to SQLAlchemyPlugin fixes SerializationException for docs May 6, 2024
Copy link
Contributor

@peterschutt peterschutt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JorenSix!

Beyond the missing serialization plugin, I like the idea of seeding the database with some test data!

We really should have a test for the example in tests/examples/test_contrib/test_sqlalchemy, would you mind adding one as part of this PR? It should hit the GET /authors endpoint and test the json response.

@JorenSix
Copy link
Contributor Author

I have modified the example using the suggestions above. The metadata is now created automatically, the data via on_startup.

I also created a new unit test file which checks if the response is 200 and a non empty list of authors is returned. This works with recent python versions.

Unfortunately this fails for older python versions (3.9, 3.10), which immediately shows why it is indeed a good idea to provide a unit test. The litestar.exceptions.base_exceptions.SerializationException looks similar to me as the original associated issue #3464

Traceback (most recent call last):
  File "/workspaces/litestar/litestar/middleware/_internal/exceptions/middleware.py", line 158, in __call__
    await self.app(scope, receive, capture_response_started)
  File "/workspaces/litestar/litestar/_asgi/asgi_router.py", line 99, in __call__
    await asgi_app(scope, receive, send)
  File "/workspaces/litestar/litestar/routes/http.py", line 80, in handle
    response = await self._get_response_for_request(
  File "/workspaces/litestar/litestar/routes/http.py", line 132, in _get_response_for_request
    return await self._call_handler_function(
  File "/workspaces/litestar/litestar/routes/http.py", line 156, in _call_handler_function
    response: ASGIApp = await route_handler.to_response(app=scope["app"], data=response_data, request=request)
  File "/workspaces/litestar/litestar/handlers/http_handlers/base.py", line 557, in to_response
    return await response_handler(app=app, data=data, request=request)  # type: ignore[call-arg]
  File "/workspaces/litestar/litestar/handlers/http_handlers/_utils.py", line 79, in handler
    return response.to_asgi_response(app=None, request=request, headers=normalize_headers(headers), cookies=cookies)  # pyright: ignore
  File "/workspaces/litestar/litestar/response/base.py", line 451, in to_asgi_response
    body=self.render(self.content, media_type, get_serializer(type_encoders)),
  File "/workspaces/litestar/litestar/response/base.py", line 392, in render
    return encode_json(content, enc_hook)
  File "/workspaces/litestar/litestar/serialization/msgspec_hooks.py", line 152, in encode_json
    raise SerializationException(str(msgspec_error)) from msgspec_error
litestar.exceptions.base_exceptions.SerializationException: Unsupported type: <class 'docs.examples.contrib.sqlalchemy.sqlalchemy_declarative_models.Book'>

Feedback or instructions on how to proceed are welcome.

As an aside: I already learned a lot about the litestar CI infra and the use of the .devcontainer folder. Very practical indeed to e.g. switch python versions and quickly run unit tests.

@Alc-Alc
Copy link
Contributor

Alc-Alc commented May 13, 2024

@JorenSix can you try this change?

books: "Mapped[list[Book]]" = relationship(back_populates="author", lazy="selectin")

which immediately shows why it is indeed a good idea to provide a unit test.

@Kumzy has a related (ish) PR #3445 here that would make sure all code examples are tested

@github-actions github-actions bot added the area/dependencies This PR involves changes to the dependencies label May 16, 2024
Copy link

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/3475

@JorenSix
Copy link
Contributor Author

Thanks for following up and the quick feedback. The process and the infrastructure of litestar seems exemplary. I learned a lot about how you handle things here. Thanks again.

@peterschutt peterschutt merged commit 5afa40f into litestar-org:main May 17, 2024
25 of 27 checks passed
@peterschutt
Copy link
Contributor

@all-contributors add @JorenSix for docs

Copy link
Contributor

@peterschutt

I've put up a pull request to add @JorenSix! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies This PR involves changes to the dependencies area/docs This PR involves changes to the documentation pr/external size: small Triage Required 🏥 This requires triage type/docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: SerializationException when running modeling-and-features demo from docs
4 participants