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

Allow Override Static URLs #9

Open
da3dsoul opened this issue Mar 6, 2019 · 11 comments
Open

Allow Override Static URLs #9

da3dsoul opened this issue Mar 6, 2019 · 11 comments

Comments

@da3dsoul
Copy link

da3dsoul commented Mar 6, 2019

I don't know what to call this.
In most languages, types are static, and you can easily say:

@route("/menu/image/<image_id>")
def get_by_id(image_id):
    pass

@route("/menu/image/unsorted")
def get_unsorted():
    pass

image_id is clearly an int or str in this context, but when we have a rule that states that get_unsorted should be called, then it should match that exact URL rather than feeding "unsorted" as a parameter to get_by_id.
Due to the fact that rules are stored as a dict, they are not predictably sorted by nature, which makes them fast, at least. When you have a situation like the above, it will randomly work or not work. If it was FIFO, then at least you could define the route that you want to be prioritized first.

The solution I propose is to get all routes that match the url, see if one matches it exactly and use that, then somehow determine which one to use instead if it doesn't match exactly and matches multiple. Ideally, it would tell the developer somehow if there are multiple matches, so developers don't need to chase a typing and regex issue like I did to find this.

@da3dsoul
Copy link
Author

da3dsoul commented Mar 7, 2019

#10 fixes this

@dagwieers
Copy link
Contributor

This would be wonderful. I also wondered why this was not possible as we needed this.
We actually had to refactor our interface (causing incompatibilities) because this wasn't possible.

@da3dsoul
Copy link
Author

My fork has other things that weren't accepted. You can use it or build your own with the little change

@dagwieers
Copy link
Contributor

Thanks for that, but we would prefer to use the original project with your fixes ;-)

@dagwieers
Copy link
Contributor

Can we have this proposed in a separate PR?
I think this one isn't controversial.

@da3dsoul
Copy link
Author

Oh fine I'll split them up into just bug fixes and the script stuff

@dagwieers
Copy link
Contributor

Whatever makes it acceptable for merging ;-)
I hate to have to see the self-evident stuff being kept hostage over the controversial ones.
It also makes it harder to review each of the features on its own merits.

@da3dsoul
Copy link
Author

da3dsoul commented Aug 25, 2019 via email

@dagwieers
Copy link
Contributor

dagwieers commented Aug 31, 2019

Still waiting for that PR. ;-)

I looked in #10 and apart from #9 (#8 not so much) I see other fixes in #10 I like and would like to see those proposed in separate PRs.

@da3dsoul
Copy link
Author

Sorry. Busy with work. I planned to do it a while ago. I'll go through each of the things I did and propose them separately. Convert Arguments as one, Script support as one, bug fixes, etc

@dagwieers
Copy link
Contributor

I don't mind helping or testing. Let me know.

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

No branches or pull requests

2 participants