-
Notifications
You must be signed in to change notification settings - Fork 46
feat: add Litestar framework support and related middleware #591
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
base: 0.30
Are you sure you want to change the base?
Conversation
|
Hi @rishabhpoddar @namsnath I have made some changes to support litestar but the tests are failing need some help here since I couldn't debug where the issue is exactly coming from can any one guide me through what am I doing wrong? |
|
📝 Documentation updates detected! New suggestion: Add Litestar framework support documentation |
|
Hey @pythonhubdev , Which test suite has the failures? Any specific tests? I can take a look at this in detail sometime next week. |
|
Hi @namsnath thanks for the response so except the top tests all other tests are failing. Mainly the tests that are calling the internal API of supertokens are failing but they are passing in FastAPI I don't know if I have to mount / attach the app or something so mostly it's 404 a little guidance help me I can fix it and the other issues are 500 Internal Server Error I don't know exactly why it's failing. |
|
Hi @namsnath I have updated the test and modified the Litestar middleware to use |
|
Hey @pythonhubdev ,
When you fix this, please also make sure the code works in the lowest supported version of Python (3.8). You seem to be running Python 3.10+. I'll add a few more review comments later today and look at the framework in detail sometime later. |
|
Hi @namsnath the major issue is I was not able to access those routes even manually not alone in testing I think the app is not getting attached or something is getting missed out due to some reason I can't access the |
|
@pythonhubdev ,
I could get the requests routed correctly with these changes in the example app: # supertokens_python/framework/litestar/litestar_request.py
class LitestarRequest(BaseRequest):
....
def get_path(self) -> str:
return self.request.scope["raw_path"].decode("utf-8")
# main.py
# TODO: Move to `litestar_middleware.py`
# If using the import from `litestar_middleware` does not work, convert to a factory function
@asgi("/auth", is_mount=True, copy_scope=True)
async def supertokens_app(scope: Scope, receive: Receive, send: Send) -> None:
# <code from `LitestarMiddleware.handle` without `next_app` handling>
# Additional changes:
asgi_response = result.response.to_asgi_response(app=None, request=request) # Removed `await`, Set `app=None`
app = Litestar(
route_handlers=[supertokens_app, ...],
# middleware=[],
exception_handlers={...},
)A few things to keep in mind for this PR:
I'll add a few items to the TODO section of the PR as well. Ref: |
|
@namsnath fixed the PR comments working on the ASGI handled you mentioned some are failing and facing some issues too will fix and update. |
|
Hey any progress on this ? Does this need to be picked up ? |
|
Hi, I may require some help in this facing various issues and things are failing |
0dac681 to
a21d10d
Compare
|
@namsnath can you please help me out in fixing this got some time I want complete this PR this week. I am getting into issues:
|
|
@namsnath @Realsid I have pushed some changes please check it out there are some obvious issues which I need help with
app = Litestar(
middleware=[get_middleware()]
)Around 16 tests are failing and mostly it's 404 not found. I don't know what exactly is the issue here but any help here would be great. |
|
Hi @pythonhubdev ,
Please look at #591 (comment) for an explanation of what's happening with the 404s. I would be interested to find out if there is a way to circumvent the exception handling and allow our middleware to work as expected. |
|
Yeah I tried out different setups but facing the same issue for one or the other routes. I will try to ask regarding this in the Litestar community for better understanding. |
|
Hi @namsnath cleanup the functions and added plugin instead of middleware to mount |
|
@pythonhubdev , a side note,could you please describe why you switched from middleware to implementing supertokens functionality as asgi app rather than middleware or you could point to that discussion. Thank you. |
|
Yeah the discussion was on discord: https://discord.com/channels/919193495116337154/1433816355177496596 |
@namsnath can you help me with this please? |
|
Hi @pythonhubdev , You can rebase your branch off the latest version branch. This has a bunch of CI improvements. Adding the run-tests label to the PR will allow you to run all the tests. |
|
Thanks @namsnath |
b1f5eac to
08248af
Compare
|
@namsnath I have rebased with branch 0.30 and pushed. Please check. Also I am not able to add / update the label. |
|
Please use 0.31 @pythonhubdev |
Sorry, 0.30 is correct Please also make sure the lowest supported Python version (3.8) works with your changes. Currently, I see a new Django version being used, which is not compatible with this. |
|
@namsnath will fix these referring to the latest version branch. |
|
@namsnath I have reverted all packages to the same version as in branch 0.30 and also I have changed to |
|
Please look at the other changes required for the changed action as well. Approving a workflow run to see if the rest of the tests run fine. |
|
@namsnath I have fixed the linting issues and also I have updated the workflow similar to other workflows as well. Can you please check? |
|
Looks like there is a dependency on litestar outside of the frameworks module. Please fix this. You can find the logs as downloadable artifacts in the failing workflow runs. Trace from a website tests run: |
|
Hi I was using a relative import that was causing the issue I guess I have fixed it. Can you please check the latest commit if the changes are fine? @namsnath |
Summary of change
(A few sentences about this PR)
Related issues
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
Documentation changes
(If relevant, please create a PR in our docs repo, or create a checklist here highlighting the necessary changes)
Checklist for important updates
coreDriverInterfaceSupported.jsonfile has been updated (if needed)supertokens_python/constants.pyfrontendDriverInterfaceSupported.jsonfile has been updated (if needed)setup.pysupertokens_python/constants.pygit tag) in the formatvX.Y.Z, and then find the latest branch (git branch --all) whoseX.Yis greater than the latest released tag.supertokens_python/utils.pyfile to include that in theFRAMEWORKSvariablesyncio/asynciofunctions are consistent.tests/sessions/test_access_token_version.pyto account for any new claims that are optional or omitted by the coreRemaining TODOs for this PR
framework/litestar/__init__.pylitestartosetup.pyMakefiletests/frontendIntegration/litestarsupertokens-websitetests run with litestartests/auth-react/litestarsupertokens-auth-reacttests run with litestar