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

Show 404 for not running/existent sandbox #224

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dobrac
Copy link

@dobrac dobrac commented Dec 19, 2024

No description provided.

@ValentaTomas
Copy link
Member

One consideration is whether we should return 404 specifically when the sandbox is not found.
The request is not to an API about a resource but to a "server" that is not accessible.

Also, because we might return 404 for some requests from the SDK (for example, if a file is not found in the sandbox), it would make it harder to distinguish between these errors.

@jakubno
Copy link
Member

jakubno commented Dec 20, 2024

Good point, but what are the options, 503 maybe? In the end, the text should explain it

@ValentaTomas
Copy link
Member

Maybe even 502 could make sense, but as you said, if we explain in the next what is happening, it should not be a problem.
I think thinking here in terms of what are the other errors we will want to return for OOM/port could make it clearer what should this one be.

Base automatically changed from dev to main January 1, 2025 17:09
@ValentaTomas ValentaTomas added the improvement Improvement for current functionality label Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement for current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants