refactor http router for better werkzeug compatibility#10011
Conversation
S3 Image Test Results (AMD64 / ARM64) 2 files 2 suites 3m 8s ⏱️ Results for commit fbf8356. ♻️ This comment has been updated with latest results. |
LocalStack Community integration with Pro 2 files 2 suites 1h 15m 53s ⏱️ Results for commit fbf8356. ♻️ This comment has been updated with latest results. |
alexrashed
left a comment
There was a problem hiding this comment.
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! 💯
| yield rule | ||
|
|
||
|
|
||
| class _EndpointFunction(RuleFactory): |
There was a problem hiding this comment.
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)?
| def remove_rule(self, rule: Rule): | ||
| """DEPRECATED: use ``remove`` instead.""" | ||
| self._remove_rules([rule]) | ||
| class _EndpointRule(RuleFactory, Generic[E]): |
There was a problem hiding this comment.
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?
| 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) |
There was a problem hiding this comment.
This is a nice encapsulation of the different possibilities of defining rules directly using Werkzeug classes! 💯
045c908 to
fbf8356
Compare
Motivation
Werkzeug has clever utilities such as
Submountwhich allows you to streamline the creation of routers:Our
@route,@resource, or endpoint concepts have not been compatible with these, because the werkzeugRuleobjects are created withinRouter.add, instead of an encapsulatingRuleFactory, which is werkzeug's mechanism of building higher-level routing utilities.This PR moves all the code that generates
Ruleobjects out ofRouter.add, and instead encapsulates them intoRuleFactoryclasses. This makes them naturally compatible with werkzeug.For instance, now you can do something like:
Where you'll be able to call:
localhost:4566/_ext/myextension/v1/hellomyextension.localhost.localstack.cloud:4566/v1/helloThis 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 vialocalhost.localstack.cloud:4566/v1/...because of potential collisions, but would be ok throughopenai.localhost...orlocalhost.localstack.cloud:4566/openai-mock/v1/....The utilities of this PR allows you to easily do that with a combination of
WithHostandSubmount, as in the example above.Changes
Ruleobjects from endpoints and other localstack concepts is now in aRuleAdapterclass that is compatible with werkzeug'sRuleFactoryRouterand refactored existing code accordingly