Handle requests without file extension#794
Conversation
…e. First commit, routing.rs.
… equivalent .sql if exists, otherwise not found).
… namespaced tests as they were getting overwhelming.
|
@guspower , I made a few comments |
…t on file_cache and filesystem, rename ExecutionStore -> FileStore as now checks both assets and sql files. *Very very rough* first implementation pass in http::main_handler. All tests pass.
|
@lovasoa very very rough first pass at integrating into main_handler. Comment most welcome. The changes to filesystem are too minimal and I need to better understand and integrate |
| serve_fallback(&mut service_request, e).await? | ||
| let app_state: &web::Data<AppState> = service_request.app_data().expect("app_state"); | ||
| let store = AppFileStore::new(&app_state.sql_file_cache, &app_state.file_system); | ||
| let path_and_query = service_request.uri().path_and_query().expect("expected valid path with query from request"); |
There was a problem hiding this comment.
we don't want to crash the app here in any case; would be nice to avoid expect.
|
Thanks a lot @guspower ! I made the required changes and merged. Feel free to open a follow-up PR if you notice I forgot something :) |
|
Ah excellent. I'll sync and take a look. w00t! :) |
|
Yep cool. Works well. The only outstanding item on my list was whether we should be returning a |
|
I think that would indeed be a nice thing. I didn't add it yesterday because the changes were large already, and that wasn't the behavior of the previous version. What do you think ? On one hand, people may already have apps that rely on 404.sql for normal pages with non-file-based URLs. On the other hand, people may already be using it to create custom error pages that should have a 404 status code and currently don't. In any case, we should avoid overriding the status code if the user sets it from their sql code (maybe use RequestContext for that). |
|
Yeah I realized there are two different requirements here (I think, maybe more!):
A few possible ways of dealing with it:
It would be really useful to know what people are using the 404.sql for! |
We already have a status_code component for this.
I think that's overkill. One can use status_code to switch between the behaviors. The only question is what should be the default one. Another interesting followup would be the ability to configure custom routes, |
|
Ah yes, custom routes, that was the whole reason I brought this up in the first place lol. Thanks for reminding me :) I'm traveling for the next couple of days but would like to have a go at a candidate implementation for that later on this week - does that work for you? Will create a new fork. |
|
Great! And if you could check "allow edits by maintainers" in your PR, that would be great! |
2 participants
replaces #786