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

server: added loading page while model loading #9401

Closed
wants to merge 18 commits into from

Conversation

VJHack
Copy link
Contributor

@VJHack VJHack commented Sep 10, 2024

Fix #9158

@VJHack
Copy link
Contributor Author

VJHack commented Sep 10, 2024

Resolves #9158

@VJHack VJHack marked this pull request as ready for review September 10, 2024 03:33
examples/server/server.cpp Outdated Show resolved Hide resolved
examples/server/server.cpp Outdated Show resolved Hide resolved
@VJHack VJHack changed the title Adding loading page for '/' server requests server: added loading page while model loading Sep 11, 2024
@VJHack VJHack changed the title server: added loading page while model loading server: added loading page while model loading (#9158) Sep 11, 2024
@github-actions github-actions bot added the python python script changes label Sep 12, 2024
Copy link
Collaborator

@ngxson ngxson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot return HTML for JSON API endpoint (please don't change the test file)

@BradHutchings
Copy link

We cannot return HTML for JSON API endpoint (please don't change the test file)

This was not the intent of the request. The intent was when an html file was requested, and the model was loading, that the server not substitute a JSON error result.

I'm sorry that I don't have experience with pull requests and couldn't submit this myself. There is a discussion of what I was trying to accomplish here:

#9158

@VJHack VJHack requested a review from ngxson September 13, 2024 03:30
@VJHack
Copy link
Contributor Author

VJHack commented Sep 13, 2024

We cannot return HTML for JSON API endpoint (please don't change the test file)

Sorry about that. There was a bit of confusion but it should be resolved now. @BradHutchings, now when you visit the page it displays the loading message without interfering with api calls that return JSON.

@ngxson, I ran the CI checks in my forked repo and they all pass.

@ngxson ngxson changed the title server: added loading page while model loading (#9158) server: added loading page while model loading Sep 13, 2024
server_state current_state = state.load();
if (current_state == SERVER_STATE_LOADING_MODEL) {
res_error(res, format_error_response("Loading model", ERROR_TYPE_UNAVAILABLE));
if(req.path == "/"){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing one requirement from the original issue #9158

Here only / is catched, not *.html. I leave a suggestion on my comment #9401 (comment) , but I'll apply it myself.

@ngxson
Copy link
Collaborator

ngxson commented Sep 13, 2024

I can't push to your repo, so I created a PR to replace this.
See #9468

@ngxson ngxson closed this Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples python python script changes server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Loading page for "/" and ".html" requests instead of json error.
4 participants