-
Notifications
You must be signed in to change notification settings - Fork 257
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
[WAIT] When processing a route, use most specific mount paths first #748
base: main
Are you sure you want to change the base?
Conversation
More specific mount paths will be found by using this order Co-Authored-By: Bruno Tremblay <[email protected]>
or use `private$mnts` where possible
private$mnts | ||
mnts <- private$mnts | ||
if (length(mnts) == 0) return(mnts) | ||
mnts[sort(names(mnts), decreasing = FALSE)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mnts[sort(names(mnts), decreasing = FALSE)] | |
mnts[order(names(mnts), decreasing = FALSE)] |
# * Mounts `/aaa` and `/aaa/bbb` exist. | ||
# * We want to use mount `/aaa/bbb` as it is more specific | ||
# TODO | ||
# * If `/aaa/bbb` mount does not support `/aaa/bbb/foo`, then try mount `/aaa`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# * If `/aaa/bbb` mount does not support `/aaa/bbb/foo`, then try mount `/aaa`. | |
# * If `/aaa/bbb` mount does not support `/aaa/bbb/foo`, then try mount `/aaa` with route `/bbb/foo`. |
@@ -242,7 +248,9 @@ Plumber <- R6Class( | |||
path <- paste0(path, "/") | |||
} | |||
|
|||
# order the mounts so that the more specific mount paths are ahead of the less specific mount paths |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# order the mounts so that the more specific mount paths are ahead of the less specific mount paths | |
# Add the mount to the original mount order | |
private$mnt_order <- c(private$mnt_order, path) | |
# Order the mounts so that the more specific mount paths are ahead of the less specific mount paths |
mnts <- private$mnts | ||
if (length(mnts) == 0) return(mnts) | ||
mnts[sort(names(mnts), decreasing = FALSE)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mnts <- private$mnts | |
if (length(mnts) == 0) return(mnts) | |
mnts[sort(names(mnts), decreasing = FALSE)] | |
private$mnts[private$mnt_order] |
This change is deemed too dangerous. Maybe approach this with cc @cpsievert |
Fixes #734
Fixes #773
Reprex:
... Which mount processes
/aaa/bbb/hello
?/aaa
/aaa/bbb
PR task list:
devtools::document()
Questions:
pr$mounts
?Answer: No. Keep
pr$mounts
the same