-
Notifications
You must be signed in to change notification settings - Fork 50
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
Add optional recursive query look up #183
base: main
Are you sure you want to change the base?
Add optional recursive query look up #183
Conversation
I personally think this should be the default (and only way) to read the queries directory. |
I'm on board with recursive query look up. Before talking about the implementation, I think we should decide if we're keeping the non-recursive version. There two minor cases where recursive lookup would be unwanted.
I'm not too concerned about these points since its unlikely that a user would have this setup in the first place, but its not entirely impossible that the user would have "regular" SQL files and cornucopia files located in children of the same folder ( A more pressing issue is, how to handle query module name conflicts? With a single directory approach, all files necessarily have distinct names (that's how filesystems work), but with a recursive approach, its possible that two different modules would end up having the same name. For example:
One way to solve this would be to use the diverging parent directory's name for disambiguating, which would yield Another potential solution is to simply give an error when a duplicate is found. Might feel restrictive and arbitrary for users, but if the error message is good, its probably the easiest solution. I'd like to resolve these questions before moving forward with this PR, so please let me know what you think 😃 |
@LouisGariepy What about creating sub-modules to reflect the real directory structure that the query files reside in? I.e. mod foo {
mod bar {
mod my_queries {
...
}
}
} |
Personally I think using sub-modules would be a solid option for resolving duplicate files. For the issue of "normal" sql files, we could use the validation of searching for |
Submodules could be a solution. The work to enable this falls a bit outside of this PR though (since its also closely related to #176 and #186), but with query submodules implemented I'd be in favor of completely replacing the current query lookup with the recursive version.
I'm not sure which way is best yet, but these are conceivable solutions. |
Sounds like a reasonable approach!
At first I wanted to vote for going the |
Changes
cli.rs
--recursive
generate_live
to look for the new flag and useread_query_modules_recursive
is set to trueread_query_modules_recursive
function as a clone ofread_query_modules
to look for directories and if they are found to runfind_queries
to locate all of the filesfind_queries
function to locate all of the files in a directory and if a directory is found, to call itself with that pathThis was something I came across when I started using cornucopia for my personal project. I wanted to try to keep my query files as close to where it was going to be used but I wasn't able to find a way to do that as it is. So I made some changes to look into the directories that are found in the given path so you can have a structure like this:
instead of:
I'm not sure if this was wanted or needed but it fits my use case and works for me so I thought I would try to share it.