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

Add support custom routes #509

Open
1 task done
antbz opened this issue Jun 1, 2023 · 3 comments · May be fixed by #862
Open
1 task done

Add support custom routes #509

antbz opened this issue Jun 1, 2023 · 3 comments · May be fixed by #862

Comments

@antbz
Copy link

antbz commented Jun 1, 2023

Checklist

  • There are no similar issues or pull requests for this yet.

Is your feature related to a problem? Please describe.

In my application, I have started using FastAPI Custom Routes to log requests and responses and add transactional headers. However, I can't use the new route class for the SQLAdmin routes because I cannot set the route class.

Describe the solution you would like.

The ability to pass route_class as an argument to the Admin.__init__() method would be a nice way to fix this issue.

I have experimented with this and I have found that while it "works" for the most part, it causes an issue with the login_required decorator.

def login_required(func: Callable[..., Any]) -> Callable[..., Any]:
    """Decorator to check authentication of Admin routes.
    If no authentication backend is setup, this will do nothing.
    """

    @functools.wraps(func)
    async def wrapper_decorator(*args: Any, **kwargs: Any) -> Any:
        admin, request = args[0], args[1]
        auth_backend = admin.authentication_backend
        if auth_backend is not None:
            response = await auth_backend.authenticate(request)
            if response and isinstance(response, Response):
                return response

        return await func(*args, **kwargs)

    return wrapper_decorator

The decorator expects two args but when using subclasses of fastapi.APIRoute, the request argument is passed as a kwarg instead.

Describe alternatives you considered

I previously used custom middleware to achieve this. I am trying to move away from it because it caused a few issues with asynchronous execution of Background Tasks. For now, I have kept the middleware just on the SQLAdmin router. This allows me to still use background tasks elsewhere and keep logs of SQLAdmin calls. I do have to support two logging mechanisms, which isn't ideal.

Additional context

No response

@aminalaee
Copy link
Owner

So if I understand correctly the line causing the issue is admin, request = args[0], args[1] right?

@antbz
Copy link
Author

antbz commented Jul 27, 2023

So if I understand correctly the line causing the issue is admin, request = args[0], args[1] right?

Yes, the function is expecting the request to be passed as a regular arg, but when subclassing APIRoute it is instead passed as a kwarg.

@aminalaee
Copy link
Owner

I think adding a check to get it from args or kwargs, and see if it solves the integration with the rest of the code.
Feel free to create a PR and contribute to this.

@hmvp hmvp linked a pull request Dec 10, 2024 that will close this issue
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 a pull request may close this issue.

2 participants