Skip to content

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
@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
@VJHack VJHack requested review from ggerganov and ngxson September 11, 2024 03:35
@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