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

Signal integration #2141

Closed
wants to merge 3 commits into from
Closed

Signal integration #2141

wants to merge 3 commits into from

Conversation

ahopkins
Copy link
Member

This is the beginning of the second stage of #1630. More particularly, it introduces a new module to Sanic internals: sanic.touchup.

The idea with this module is that there will be predefined comment/markers that will modify executable functions at startup.


For example, the code to run request middleware looks like this:

            for middleware in applicable_middleware:
                # ODE: http.middleware.before {"request": request}
                response = middleware(request)
                if isawaitable(response):
                    response = await response
                # ODE: http.middleware.after {"request": request, "response": response}  # noqa
                if response:
                    return response

At startup, there is a predefined interface that will lookup whether or not the http.middleware.before or http.middleware.after signals have been implemented. If not, then nothing happens. If yes, then the touchup module will inject signal dispatchers in those locations.

This should allow Sanic developers to make good use of signals to inject functionality, but not to slow down the req/resp cycle needlessly.


TODO:

  • Conditional block scheme
    # CB: start some_expression
    do_something_only_if_startup_condition_exists()
    # CB: end
  • unit tests

@ahopkins ahopkins requested a review from a team as a code owner May 23, 2021 21:51
@ahopkins ahopkins marked this pull request as draft May 23, 2021 21:51
@ahopkins
Copy link
Member Author

This pull request has been mentioned on Sanic Community Discussion. There might be relevant details there:

https://community.sanicframework.org/t/signals-integrations/824/4

@ahopkins
Copy link
Member Author

Scapping this idea (for now) and going to use AST for signals. I still want to explore something like this for conditional execution though.

@ahopkins ahopkins closed this Jun 13, 2021
@ahopkins ahopkins deleted the signal-integration branch June 13, 2021 05:37
@ahopkins ahopkins mentioned this pull request Jun 13, 2021
17 tasks
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 this pull request may close these issues.

1 participant