Refactor RestHandler state machine into a coroutine #21787
Conversation
…actor-resthandler-state-machine-into-coro
- Temporarily disabled LogContext (ScopedValue in rest handler state machine) - Made more stuff async - Fixed an issue by moving a coro from Future to async, because futures don't pass ExecContext as coros (just as an awaitable) - Fixed an issue by moving WAITING<->coro adapter further up the stack
…actor-resthandler-state-machine-into-coro
…e+waitForFuture The following handlers are changed: - RestAgencyHandler - RestAdminLogHandler - RestDocumentHandler - RestGraphHandler - RestImportHandler - RestLogHandler - RestLogInternalHandler - RestMetricsHandler - RestReplicationHandler - RestTransactionHandler - RestUsageMetricsHandler
…cute+waitForFuture
…actor-rest-handlers-into-coroutines
…actor-resthandler-state-machine-into-coro
…ub.com:arangodb/arangodb into feature/refactor-resthandler-state-machine-into-coro
…actor-resthandler-state-machine-into-coro
…actor-resthandler-state-machine-into-coro
There was a problem hiding this comment.
This looks overall very good and is going exactly in the right direction. I have added some minor comments, some of which need addressing. And a CHANGELOG entry is missing. In particular I checked that the log context is now in the async context and will thus be moved from thread local variable to thread local variable if a coroutine is suspended on one thread and resumed on another. Therefore it is now allowed to use ScopedValue and log contexts in coroutines (with its lifetime bound to the coroutine context object).
| add_library(arango_async INTERFACE) | ||
| target_include_directories(arango_async INTERFACE | ||
| add_library(arango_async STATIC | ||
| include/Async/async.h |
There was a problem hiding this comment.
Why are these header files listed here? Usually, we seem to only add the .cpp files.
There was a problem hiding this comment.
They are dependencies, the library needs to be rebuilt if the headers change. That being said, I think the build system does detect them as such anyway, so it shouldn't be necessary for that. Where it does make a difference is for IDEs like Visual Studio, when the CMake files are used as a basis for a project. In particular if the header files don't have a corresponding source file of the same name, in which case they should at least be detected by heuristics.
All in all, while I don't think it's hugely important, I do think it's generally preferable to add header files as well.
Co-authored-by: Julia Volmer <julia.volmer@arangodb.com>
Co-authored-by: Julia Volmer <julia.volmer@arangodb.com>
…actor-resthandler-state-machine-into-coro
…actor-resthandler-state-machine-into-coro
Scope & Purpose
Refactor the RestHandler's state machine into a coroutine. This is foundational work to allow rewriting more code into (asynchronous) coroutines.
Checklist