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

feat: throw an error when there is not initialized database loaded yet #299

Conversation

fengelniederhammer
Copy link
Contributor

resolves #295

When SILO is still in it's configurable startup time, it will return:

HTTP/1.1 503 Service Unavailable
Date: Mon, 26 Feb 2024 11:20:53 GMT
Connection: Close
Content-Type: application/json
Retry-After: 118

{
  "error": "Service temporarily unavailable",
  "message": "Database not initialized yet. Please try again after 118 seconds."
}
``

@fengelniederhammer fengelniederhammer force-pushed the 295-throw-an-error-when-there-is-no-database-loaded-yet branch from 37ff48f to ccff1c2 Compare February 26, 2024 11:26
@fengelniederhammer fengelniederhammer force-pushed the 295-throw-an-error-when-there-is-no-database-loaded-yet branch from ccff1c2 to 19a8c40 Compare February 26, 2024 11:54
@Taepper Taepper self-assigned this Feb 26, 2024
return;
}
} catch (const silo_api::UninitializedDatabaseException& exception) {
SPDLOG_DEBUG("No database loaded yet - continuing to load initial database.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

"continuing to load ..." can be confusing for users, if the database is currently not loading an existing database state

Copy link
Collaborator

Choose a reason for hiding this comment

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

(i.e. the maintainer / user thinks it is currently loading but in fact it still waits for data, maybe because of incorrect setup)

response.setStatusAndReason(Poco::Net::HTTPResponse::HTTP_SERVICE_UNAVAILABLE);
std::string message = "Database not initialized yet.";

const auto retry_after = computeRetryAfter();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we name the method computeRetryAfter better to something that makes it clear that it refers to 1. only the uninitialized error (other errors could in theory also have a retry) and 2. it is only a hint for the user?

@Taepper Taepper removed their assignment Feb 26, 2024
@Taepper Taepper self-requested a review February 26, 2024 12:30
@fengelniederhammer fengelniederhammer merged commit b17f72a into main Feb 26, 2024
6 checks passed
@fengelniederhammer fengelniederhammer deleted the 295-throw-an-error-when-there-is-no-database-loaded-yet branch February 26, 2024 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Throw an error when there is no database loaded yet
2 participants