Skip to content

Commit

Permalink
Add runtime size configuration feature (#5805)
Browse files Browse the repository at this point in the history
Co-authored-by: openhands <[email protected]>
Co-authored-by: amanape <[email protected]>
  • Loading branch information
3 people authored Jan 6, 2025
1 parent 8cfcdd7 commit 1f8a018
Show file tree
Hide file tree
Showing 12 changed files with 274 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { screen } from "@testing-library/react";
import { describe, it, expect } from "vitest";
import { renderWithProviders } from "test-utils";
import { RuntimeSizeSelector } from "#/components/shared/modals/settings/runtime-size-selector";

const renderRuntimeSizeSelector = () =>
renderWithProviders(<RuntimeSizeSelector isDisabled={false} />);

describe("RuntimeSizeSelector", () => {
it("should show both runtime size options", () => {
renderRuntimeSizeSelector();
// The options are in the hidden select element
const select = screen.getByRole("combobox", { hidden: true });
expect(select).toHaveValue("1");
expect(select).toHaveDisplayValue("1x (2 core, 8G)");
expect(select.children).toHaveLength(3); // Empty option + 2 size options
});

it("should show the full description text for disabled options", async () => {
renderRuntimeSizeSelector();

// Click the button to open the dropdown
const button = screen.getByRole("button", {
name: "1x (2 core, 8G) SETTINGS_FORM$RUNTIME_SIZE_LABEL",
});
button.click();

// Wait for the dropdown to open and find the description text
const description = await screen.findByText(
"Runtime sizes over 1 are disabled by default, please contact [email protected] to get access to larger runtimes.",
);
expect(description).toBeInTheDocument();
expect(description).toHaveClass("whitespace-normal", "break-words");
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { screen, fireEvent } from "@testing-library/react";
import { describe, it, expect, vi } from "vitest";
import { renderWithProviders } from "test-utils";
import { createRoutesStub } from "react-router";
import { DEFAULT_SETTINGS } from "#/services/settings";
import { SettingsForm } from "#/components/shared/modals/settings/settings-form";
import OpenHands from "#/api/open-hands";

describe("SettingsForm", () => {
const getConfigSpy = vi.spyOn(OpenHands, "getConfig");
getConfigSpy.mockResolvedValue({
APP_MODE: "saas",
GITHUB_CLIENT_ID: "123",
POSTHOG_CLIENT_KEY: "123",
});

const RouterStub = createRoutesStub([
{
Component: () => (
<SettingsForm
settings={DEFAULT_SETTINGS}
models={[]}
agents={[]}
securityAnalyzers={[]}
onClose={() => {}}
/>
),
path: "/",
},
]);

it("should not show runtime size selector by default", () => {
renderWithProviders(<RouterStub />);
expect(screen.queryByText("Runtime Size")).not.toBeInTheDocument();
});

it("should show runtime size selector when advanced options are enabled", async () => {
renderWithProviders(<RouterStub />);
const advancedSwitch = screen.getByRole("switch", {
name: "SETTINGS_FORM$ADVANCED_OPTIONS_LABEL",
});
fireEvent.click(advancedSwitch);
await screen.findByText("SETTINGS_FORM$RUNTIME_SIZE_LABEL");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function AdvancedOptionSwitch({
<Switch
isDisabled={isDisabled}
name="use-advanced-options"
isSelected={showAdvancedOptions}
defaultSelected={showAdvancedOptions}
onValueChange={setShowAdvancedOptions}
classNames={{
thumb: cn(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { useTranslation } from "react-i18next";
import { Select, SelectItem } from "@nextui-org/react";

interface RuntimeSizeSelectorProps {
isDisabled: boolean;
defaultValue?: number;
}

export function RuntimeSizeSelector({
isDisabled,
defaultValue,
}: RuntimeSizeSelectorProps) {
const { t } = useTranslation();

return (
<fieldset className="flex flex-col gap-2">
<label
htmlFor="runtime-size"
className="font-[500] text-[#A3A3A3] text-xs"
>
{t("SETTINGS_FORM$RUNTIME_SIZE_LABEL")}
</label>
<Select
id="runtime-size"
name="runtime-size"
defaultSelectedKeys={[String(defaultValue || 1)]}
isDisabled={isDisabled}
aria-label={t("SETTINGS_FORM$RUNTIME_SIZE_LABEL")}
classNames={{
trigger: "bg-[#27272A] rounded-md text-sm px-3 py-[10px]",
}}
>
<SelectItem key="1" value={1}>
1x (2 core, 8G)
</SelectItem>
<SelectItem
key="2"
value={2}
isDisabled
classNames={{
description:
"whitespace-normal break-words min-w-[300px] max-w-[300px]",
base: "min-w-[300px] max-w-[300px]",
}}
description="Runtime sizes over 1 are disabled by default, please contact [email protected] to get access to larger runtimes."
>
2x (4 core, 16G)
</SelectItem>
</Select>
</fieldset>
);
}
29 changes: 21 additions & 8 deletions frontend/src/components/shared/modals/settings/settings-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ import { ModalBackdrop } from "../modal-backdrop";
import { ModelSelector } from "./model-selector";
import { useSaveSettings } from "#/hooks/mutation/use-save-settings";

import { RuntimeSizeSelector } from "./runtime-size-selector";
import { useConfig } from "#/hooks/query/use-config";

interface SettingsFormProps {
disabled?: boolean;
settings: Settings;
Expand All @@ -40,6 +43,7 @@ export function SettingsForm({
}: SettingsFormProps) {
const { mutateAsync: saveSettings } = useSaveSettings();
const endSession = useEndSession();
const { data: config } = useConfig();

const location = useLocation();
const { t } = useTranslation();
Expand Down Expand Up @@ -97,6 +101,8 @@ export function SettingsForm({
posthog.capture("settings_saved", {
LLM_MODEL: newSettings.LLM_MODEL,
LLM_API_KEY: newSettings.LLM_API_KEY ? "SET" : "UNSET",
REMOTE_RUNTIME_RESOURCE_FACTOR:
newSettings.REMOTE_RUNTIME_RESOURCE_FACTOR,
});
};

Expand All @@ -122,6 +128,8 @@ export function SettingsForm({
}
};

const isSaasMode = config?.APP_MODE === "saas";

return (
<div>
<form
Expand Down Expand Up @@ -164,16 +172,21 @@ export function SettingsForm({
isSet={settings.LLM_API_KEY === "SET"}
/>

{showAdvancedOptions && (
<AgentInput
isDisabled={!!disabled}
defaultValue={settings.AGENT}
agents={agents}
/>
)}

{showAdvancedOptions && (
<>
<AgentInput
isDisabled={!!disabled}
defaultValue={settings.AGENT}
agents={agents}
/>

{isSaasMode && (
<RuntimeSizeSelector
isDisabled={!!disabled}
defaultValue={settings.REMOTE_RUNTIME_RESOURCE_FACTOR}
/>
)}

<SecurityAnalyzerInput
isDisabled={!!disabled}
defaultValue={settings.SECURITY_ANALYZER}
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/hooks/query/use-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ const getSettingsQueryFn = async () => {
CONFIRMATION_MODE: apiSettings.confirmation_mode,
SECURITY_ANALYZER: apiSettings.security_analyzer,
LLM_API_KEY: apiSettings.llm_api_key,
REMOTE_RUNTIME_RESOURCE_FACTOR:
apiSettings.remote_runtime_resource_factor,
};
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/hooks/use-maybe-migrate-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import React from "react";
import { useSettingsUpToDate } from "#/context/settings-up-to-date-context";
import {
DEFAULT_SETTINGS,
getCurrentSettingsVersion,
DEFAULT_SETTINGS,
getLocalStorageSettings,
} from "#/services/settings";
import { useSaveSettings } from "./mutation/use-save-settings";
Expand Down
14 changes: 14 additions & 0 deletions frontend/src/i18n/translation.json
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,20 @@
"fr": "Réinitialiser aux valeurs par défaut",
"tr": "Varsayılanlara Sıfırla"
},
"SETTINGS_FORM$RUNTIME_SIZE_LABEL": {
"en": "Runtime Settings",
"zh-CN": "运行时设置",
"de": "Laufzeiteinstellungen",
"ko-KR": "런타임 설정",
"no": "Kjøretidsinnstillinger",
"zh-TW": "運行時設定",
"it": "Impostazioni Runtime",
"pt": "Configurações de Runtime",
"es": "Configuración de Runtime",
"ar": "إعدادات وقت التشغيل",
"fr": "Paramètres d'exécution",
"tr": "Çalışma Zamanı Ayarları"
},
"CONFIGURATION$SETTINGS_NEED_UPDATE_MESSAGE": {
"en": "We've changed some settings in the latest update. Take a minute to review.",
"de": "Mit dem letzten Update haben wir ein paar Einstellungen geändert. Bitte kontrollieren Ihre Einstellungen.",
Expand Down
10 changes: 10 additions & 0 deletions frontend/src/services/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export type Settings = {
LLM_API_KEY: string | null;
CONFIRMATION_MODE: boolean;
SECURITY_ANALYZER: string;
REMOTE_RUNTIME_RESOURCE_FACTOR: number;
};

export type ApiSettings = {
Expand All @@ -18,6 +19,7 @@ export type ApiSettings = {
llm_api_key: string | null;
confirmation_mode: boolean;
security_analyzer: string;
remote_runtime_resource_factor: number;
};

export const DEFAULT_SETTINGS: Settings = {
Expand All @@ -28,6 +30,7 @@ export const DEFAULT_SETTINGS: Settings = {
LLM_API_KEY: null,
CONFIRMATION_MODE: false,
SECURITY_ANALYZER: "",
REMOTE_RUNTIME_RESOURCE_FACTOR: 1,
};

export const getCurrentSettingsVersion = () => {
Expand Down Expand Up @@ -66,10 +69,17 @@ export const getLocalStorageSettings = (): Settings => {
LLM_API_KEY: llmApiKey || DEFAULT_SETTINGS.LLM_API_KEY,
CONFIRMATION_MODE: confirmationMode || DEFAULT_SETTINGS.CONFIRMATION_MODE,
SECURITY_ANALYZER: securityAnalyzer || DEFAULT_SETTINGS.SECURITY_ANALYZER,
REMOTE_RUNTIME_RESOURCE_FACTOR:
DEFAULT_SETTINGS.REMOTE_RUNTIME_RESOURCE_FACTOR,
};
};

/**
* Get the default settings
*/
export const getDefaultSettings = (): Settings => DEFAULT_SETTINGS;

/**
* Get the current settings, either from local storage or defaults
*/
export const getSettings = (): Settings => getLocalStorageSettings();
7 changes: 7 additions & 0 deletions openhands/server/routes/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ async def store_settings(
# LLM key isn't on the frontend, so we need to keep it if unset
if settings.llm_api_key is None:
settings.llm_api_key = existing_settings.llm_api_key

# Update sandbox config with new settings
if settings.remote_runtime_resource_factor is not None:
config.sandbox.remote_runtime_resource_factor = (
settings.remote_runtime_resource_factor
)

await settings_store.store(settings)

return JSONResponse(
Expand Down
1 change: 1 addition & 0 deletions openhands/server/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ class Settings:
llm_model: str | None = None
llm_api_key: str | None = None
llm_base_url: str | None = None
remote_runtime_resource_factor: int | None = None
85 changes: 85 additions & 0 deletions tests/unit/test_settings_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
from unittest.mock import AsyncMock, MagicMock, patch

import pytest
from fastapi.testclient import TestClient

from openhands.core.config.sandbox_config import SandboxConfig
from openhands.server.app import app
from openhands.server.settings import Settings


@pytest.fixture
def test_client():
# Mock the middleware that adds github_token
class MockMiddleware:
def __init__(self, app):
self.app = app

async def __call__(self, scope, receive, send):
if scope['type'] == 'http':
scope['state'] = {'github_token': 'test-token'}
await self.app(scope, receive, send)

# Replace the middleware
app.middleware_stack = None # Clear existing middleware
app.add_middleware(MockMiddleware)

return TestClient(app)


@pytest.fixture
def mock_settings_store():
with patch('openhands.server.routes.settings.SettingsStoreImpl') as mock:
store_instance = MagicMock()
mock.get_instance = AsyncMock(return_value=store_instance)
store_instance.load = AsyncMock()
store_instance.store = AsyncMock()
yield store_instance


@pytest.mark.asyncio
async def test_settings_api_runtime_factor(test_client, mock_settings_store):
# Mock the settings store to return None initially (no existing settings)
mock_settings_store.load.return_value = None

# Test data with remote_runtime_resource_factor
settings_data = {
'language': 'en',
'agent': 'test-agent',
'max_iterations': 100,
'security_analyzer': 'default',
'confirmation_mode': True,
'llm_model': 'test-model',
'llm_api_key': None,
'llm_base_url': 'https://test.com',
'remote_runtime_resource_factor': 2,
}

# The test_client fixture already handles authentication

# Make the POST request to store settings
response = test_client.post('/api/settings', json=settings_data)
assert response.status_code == 200

# Verify the settings were stored with the correct runtime factor
stored_settings = mock_settings_store.store.call_args[0][0]
assert stored_settings.remote_runtime_resource_factor == 2

# Mock settings store to return our settings for the GET request
mock_settings_store.load.return_value = Settings(**settings_data)

# Make a GET request to retrieve settings
response = test_client.get('/api/settings')
assert response.status_code == 200
assert response.json()['remote_runtime_resource_factor'] == 2

# Verify that the sandbox config gets updated when settings are loaded
with patch('openhands.server.shared.config') as mock_config:
mock_config.sandbox = SandboxConfig()
response = test_client.get('/api/settings')
assert response.status_code == 200

# Verify that the sandbox config was updated with the new value
mock_settings_store.store.assert_called()
stored_settings = mock_settings_store.store.call_args[0][0]
assert stored_settings.remote_runtime_resource_factor == 2

0 comments on commit 1f8a018

Please sign in to comment.