V2routing

[ahopkins]

This is a pretty thorough overhaul of how the dynamic paths generated. The concept is largely still the same, but with this methodology we can take out some of the more hacky bits re: indentations. Also, in my testing it covers a wider array of edge cases including #35.

This also closes the loop on a few items including:

• Proper differentiation between alpha and string param types
• Adds a slug param type
• Deprecates <foo:string> in favor of <foo:str>
• Deprecates <foo:number> in favor of <foo:float>
• Adds a route.uri accessor

Fixes #35
Fixes #32
Resolves #34
Resolves #12
Resolves #11
Closes #28

[ashleysommer]

Awesome! I'll check out the new implementation.

[ahopkins]

@sanic-org/framework thoughts?

I'd like to release this as a rc, and as a full version with 21.6

[Tronic]

That review was only for the docs, not the other changes (not sure how I approving 2 reviewed files ended up approving 8 others without Github showing them to me).

[Tronic]

Is the path regex correct? What's that trickery with optional initial non-slash?

[ahopkins]

We have some tests for this here: https://github.com/sanic-org/sanic/blob/main/tests/test_routes.py#L582

TBH, I cannot recall the reason for making it optional. The commit is here: 87d8ada

It does seem like we can probably just remove it completely. Will need to do more testing though.

[Tronic]

What is at index 0 of that map? String type, presumably, but a comment here would be helpful.

[ahopkins]

I think if the code is reworked slightly it becomes self-documenting:

        is_regex = label not in self.router.regex_types
        priority = (
            0
            if is_regex
            else list(self.router.regex_types.keys()).index(label)
        )
        self._params[idx] = ParamInfo(
            name, raw_path, label, cast, pattern, is_regex, priority
        )

[Tronic]

Reviewed everything except the code generation in tree.py. Mostly stylistic issues, possibly minor bugs but no algorithmic problems that I could find so far.

I'd suggest implementing a helper class for code generation that internally keeps track of indentation to avoid manual indent management and all the clutter that comes with Line objects etc.

[Tronic]

💯

[ahopkins]

Reviewed everything except the code generation in tree.py. Mostly stylistic issues, possibly minor bugs but no algorithmic problems that I could find so far.

I'd suggest implementing a helper class for code generation that internally keeps track of indentation to avoid manual indent management and all the clutter that comes with Line objects etc.

What do you have in mind:

source.add("#code at same indent")
source.add("#code indent one", 1)
source.add("#code dedent one", -1)
source.add("#code specifically at indentation=1", at=1)

Not a bad idea, because we could do entire blocks:

source.add(f"""
try:
    something({args})
except Exception:
    ...
""")

I did have a version of it using the ast module directly that basically did this. But, I think it is much easier to follow using string source.

---

I think this is worthy of a ticket for future refactor, not now.

[Tronic]

Other than the odd ellipsis, looks good.

[Tronic]

What is this?

[Tronic]

Re code generation: maybe take that a step forward and use actual inline Python code instead of string literals. Code that is syntactically valid can be loaded even if the variables don't exist, provided that the particular path never gets executed. Annotations or some other hacks could be used to inject parts into it. Well, you get the idea but this is becoming some true black magic by now.

# This is router code
if need_foo:
    with codegen:
        # This is generated code
        if route == "foo":
            return handler_foo

[ahopkins]

Strings allow interpolation. If we prefer a code based option, I'd be more favorable towards AST. An option worth keeping on the table.