Redis timeouts #13
Redis timeouts #13
Conversation
tests/test_redis_timeout.py
Outdated
| from server import Settings | ||
|
|
||
| # Create Redis client with timeout configuration | ||
| settings = Settings(redis_url="redis://localhost:6379/0", ttl=60 * 60) |
There was a problem hiding this comment.
This is an example of the LLM testing our dependency instead of testing our own library. All we're doing here is testing the behavior of redis. We put a URL in the Settings object here and then immediately pull it out. Nothing else here touches our code. I don't think we need to burden our prototype or tiled itself with these tests.
It's useful to confirm that this works—once—but I would say it's the redis library's job to unit test it.
There was a problem hiding this comment.
Yep, I was aware of that. I wanted to see the behavior of the Redis client. I also wanted to document my reasoning for the socket_timeout and show how it resolves the problem. I was hoping to avoid the situation where a reviewer says, "are you sure we need that socket_timeout", and then I have to write some code to show why we need socket_timeout. I definitely don't think the test is something we would merge into tiled. And if you don't think it should be part of the prototype I can remove it.
The Redis connection by default waits indefinitely for operations.
I added a test to check this behavior, and then added socket_timeout kwarg for the Redis client on the server to set the timeout.
The Redis client will now raise a TimeoutError if an operation takes too long.
Tiled provides a
unhandled_exception_handlerwhich should report a 500 error to the client and write the exception and stack trace to the logs https://github.com/bluesky/tiled/blob/de3583c96ccda2b732db0a84cd410367f16b94f0/tiled/server/app.py#L333