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

Add ability to define custom runtime classes #6955

Merged
merged 3 commits into from
Feb 26, 2025

Conversation

raymyers
Copy link
Contributor

@raymyers raymyers commented Feb 26, 2025

  • This change is worth documenting at https://docs.all-hands.dev/
  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

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, overriding on_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:

#  Default:
# runtime = "docker"

runtime = "app.mystuff.CustomRuntime"

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 the E2Box class before, a type error which probably did not work. It's now mapped to E2BRuntime.

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.

@raymyers raymyers force-pushed the ray/custom-runtime-classes branch from 818beeb to 688ad19 Compare February 26, 2025 04:16
@enyst
Copy link
Collaborator

enyst commented Feb 26, 2025

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"
Copy link
Contributor Author

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).

python/mypy#4717

@raymyers
Copy link
Contributor Author

raymyers commented Feb 26, 2025

@enyst

This PR seems related maybe, WDYT?

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".

But given that it's similar class selection config some consistency would be nice. A way I could make this one more consistent would be use only the existing runtime config var, and say if you don't specify one of the known values you can use a class name. (Edit: This simplification is done)

@enyst
Copy link
Collaborator

enyst commented Feb 26, 2025

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.

Copy link
Collaborator

@rbren rbren left a comment

Choose a reason for hiding this comment

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

This seems useful!

@enyst enyst merged commit db1f5a8 into All-Hands-AI:main Feb 26, 2025
14 checks passed
@raymyers raymyers deleted the ray/custom-runtime-classes branch February 26, 2025 15:46
adityasoni9998 pushed a commit to adityasoni9998/OpenHands that referenced this pull request Mar 3, 2025
Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>
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.

3 participants