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 an API to allow passing objects to middleware #120

Closed
craigmulligan opened this issue Nov 19, 2019 · 4 comments
Closed

Add an API to allow passing objects to middleware #120

craigmulligan opened this issue Nov 19, 2019 · 4 comments
Labels
enhancement New feature or request

Comments

@craigmulligan
Copy link
Contributor

craigmulligan commented Nov 19, 2019

I'm adding rele to flask app, and the subscription callbacks need the flask app_context. I've done this by way of middleware:

eg:

class FlaskMiddleware(BaseMiddleware):
    def pre_process_message(self, subscription, message):
        from server import app
        self.ctx = app.app_context()
        self.ctx.push()

    def post_process_message(self):
        self.ctx.pop()

But to make this reusable across our services and other flask apps, I'd need a way to add arbitrary data to the config that is passed to middleware.setup method or have an easy way to call custom middleware functions.

eg:

class FlaskMiddleware(BaseMiddleware):
    def setup(self, config):
        self.app = config["FLASK_APP"]

    def pre_process_message(self, subscription, message):
        self.ctx = self.app.app_context()
        self.ctx.push()

    def post_process_message(self):
        self.ctx.pop()
@andrewgy8
Copy link
Contributor

andrewgy8 commented Nov 19, 2019

Flask integration sounds like a great addition and natural progression. 👏

If I understand correctly though, the real problem to solve here would be adding extra data to the Config class. In this case it would be the Flask App.

One solution that came to mind, and I'm not sure how elegant it is, would be to add config.extras which would be a way to add whatever you want to the configuration. I'm not super happy with that solution, as the config could get very messy, unorganized, and unpredictable.

Another way to solve this issue would be to add args/kwargs to the config.setup method, passing them through to the register_middleware and then to the BaseMiddleware.setup. That would allow anyone calling setup to configure their app accordingly. Better, but still a bit convoluted.

I was looking at how Celery does it, and to be honest its not pretty.

These are some thoughts on this, and perhaps someone else has a better solution.

@andrewgy8 andrewgy8 added the enhancement New feature or request label Nov 19, 2019
@tobami
Copy link
Contributor

tobami commented Nov 19, 2019

middleware.setup() gets the config object as an argument, so what you describe can be implemented exactly that way.

@craigmulligan
Copy link
Contributor Author

@andrewgy8 yea that's exactly the usecase. I think passing kwargs + config to setup function is probably the cleanest.

@tobami, unfortunately not as Config is strict with what takes from settings https://github.com/mercadona/rele/blob/master/rele/config.py#L10

@andrewgy8
Copy link
Contributor

Closing as it was addressed in #123 and will be in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants