-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add ability to define custom runtime classes #6955
Conversation
818beeb
to
688ad19
Compare
This PR seems related maybe, WDYT? |
from openhands.runtime.impl.local.local_runtime import LocalRuntime | ||
from openhands.runtime.impl.modal.modal_runtime import ModalRuntime | ||
from openhands.runtime.impl.remote.remote_runtime import RemoteRuntime | ||
from openhands.runtime.impl.runloop.runloop_runtime import RunloopRuntime | ||
from openhands.utils.import_utils import get_impl | ||
|
||
# mypy: disable-error-code="type-abstract" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting feature gap behind this type ignore (needless rabbithole though).
Neat, I hadn't seen that one yet. It's related but a bit different, because what it's refactoring into ActionExecuter is associated with ActionExecutionServer, and the Runtime classes are ActionExecutionClient which calls it. So that PR impacts "what do you run", and this one is about "where do you run it".
|
I think this is a nice PR, thank you! I particularly like how the implementation keeps things readable, and you are entirely correct, consistency with the other PR is really good to have. I'd love to know how @rbren thinks about this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems useful!
Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>
End-user friendly description of the problem this fixes or functionality that this introduces.
Allow advanced users to define their own custom runtimes.
Give a summary of what the PR does, explaining any non-trivial design decisions.
The motivating use case is to allow hosted implementations to watch events for monitoring purposes, without depending on a monitoring SDK in this codebase. (First try PR, with some discussion). That leads to a desire to subclass a
Runtime
implementation, overridingon_event
to add monitoring.However, currently only a hard-coded mapping of existing Runtime classes may be configured. This PR allows a fully-qualified class-name to be specified instead. For example:
I've chosen to only document this new option in the
runtime
module README, not the config template.Change: The
e2b
runtime name was returning theE2Box
class before, a type error which probably did not work. It's now mapped toE2BRuntime
.Future: If we wanted, this would allow us to move some of the existing runtime implementations from parts of the codebase into usage examples, so we do not have to make every user depend multiple vendor SDKs they aren't using.
Link of any specific issues this addresses.