Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

refactor http router for better werkzeug compatibility#10011

Merged
thrau merged 2 commits into
masterfrom
router-improvements
Jan 11, 2024
Merged

refactor http router for better werkzeug compatibility#10011
thrau merged 2 commits into
masterfrom
router-improvements

Conversation

@thrau

@thrau thrau commented Jan 7, 2024

Copy link
Copy Markdown
Member

Motivation

Werkzeug has clever utilities such as Submount which allows you to streamline the creation of routers:

url_map = Map([
    Rule('/', endpoint='index'),
    Submount('/blog', [
        Rule('/', endpoint='blog/index'),
        Rule('/entry/<entry_slug>', endpoint='blog/show')
    ])
])

Our @route, @resource, or endpoint concepts have not been compatible with these, because the werkzeug Rule objects are created within Router.add, instead of an encapsulating RuleFactory, which is werkzeug's mechanism of building higher-level routing utilities.

This PR moves all the code that generates Rule objects out of Router.add, and instead encapsulates them into RuleFactory classes. This makes them naturally compatible with werkzeug.

For instance, now you can do something like:

@route("/v1/hello", methods=["GET"])
def hello(request: Request, _args):
    return "hello"

@route("/v1/world", methods=["GET"])
def world(request: Request, _args):
    return "world"

def main():
    router = Router()

    api = RuleAdapter([
        hello,
        world,
    ])
    
    router.add([
        WithHost("myextension.localhost.localstack.cloud:4566", [api]),
        Submount("/_ext/myextension", [api]),
    ])

Where you'll be able to call:

  • localhost:4566/_ext/myextension/v1/hello
  • myextension.localhost.localstack.cloud:4566/v1/hello

This is going to be very useful to implement higher-level patterns like in extensions. For example, for the openai mock extension localstack/localstack-extensions#36, there are several routes that all have hardcoded routes prefixed with /v1/.... We wouldn't want to expose this via localhost.localstack.cloud:4566/v1/... because of potential collisions, but would be ok through openai.localhost... or localhost.localstack.cloud:4566/openai-mock/v1/....
The utilities of this PR allows you to easily do that with a combination of WithHost and Submount, as in the example above.

Changes

  • the logic to create werkzeug Rule objects from endpoints and other localstack concepts is now in a RuleAdapter class that is compatible with werkzeug's RuleFactory
  • Removed deprecated methods from Router and refactored existing code accordingly

@thrau thrau added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Jan 7, 2024
@github-actions

github-actions Bot commented Jan 7, 2024

Copy link
Copy Markdown

S3 Image Test Results (AMD64 / ARM64)

  2 files    2 suites   3m 8s ⏱️
386 tests 336 ✅  50 💤 0 ❌
772 runs  672 ✅ 100 💤 0 ❌

Results for commit fbf8356.

♻️ This comment has been updated with latest results.

@coveralls

coveralls commented Jan 8, 2024

Copy link
Copy Markdown

Coverage Status

coverage: 83.904% (+0.001%) from 83.903%
when pulling fbf8356 on router-improvements
into 29d1995 on master.

@github-actions

github-actions Bot commented Jan 8, 2024

Copy link
Copy Markdown

LocalStack Community integration with Pro

    2 files      2 suites   1h 15m 53s ⏱️
2 579 tests 2 347 ✅ 232 💤 0 ❌
2 580 runs  2 347 ✅ 233 💤 0 ❌

Results for commit fbf8356.

♻️ This comment has been updated with latest results.

@alexrashed alexrashed left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wow, really nice refactoring! This is a really great enhancement, unlocking quite some possibilities to use more features from Werkzeug directly! This will definitely simplify the rule definition and increase their readibility! 💯

Comment thread localstack/http/router.py
yield rule


class _EndpointFunction(RuleFactory):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe you could add a small docstring explaining that this class is used for callables which have been decorated with the route decorator (which will take care of setting the expected rule_attributes as a member of the callable object)?

Comment thread localstack/http/router.py
def remove_rule(self, rule: Rule):
"""DEPRECATED: use ``remove`` instead."""
self._remove_rules([rule])
class _EndpointRule(RuleFactory, Generic[E]):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a nice encapsulation of the creation of rules based on the rule attributes (which was previously in the _add_endpoint function). Maybe you could also add a small docstring here?

Comment thread localstack/http/router.py
Comment on lines +472 to +483
if "path" in kwargs or type(args[0]) is str:
self.factory = _EndpointRule(*args, **kwargs)
elif "fn" in kwargs or callable(args[0]):
self.factory = _EndpointFunction(*args, **kwargs)
elif "rule_factory" in kwargs:
self.factory = kwargs["rule_factory"]
elif isinstance(args[0], RuleFactory):
self.factory = args[0]
elif isinstance(args[0], list):
self.factory = RuleGroup([RuleAdapter(rule) for rule in args[0]])
else:
self.factory = _EndpointsObject(*args, **kwargs)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a nice encapsulation of the different possibilities of defining rules directly using Werkzeug classes! 💯

@thrau thrau force-pushed the router-improvements branch from 045c908 to fbf8356 Compare January 11, 2024 12:25
@thrau thrau merged commit 1a6dec8 into master Jan 11, 2024
@thrau thrau deleted the router-improvements branch January 11, 2024 16:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants