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

Partial update service #621

Open
wants to merge 15 commits into
base: fix/meme-staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions api.md
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,64 @@ Update service configuration `service_config_id` with the provided template.

---

### `PATCH /api/v2/service/{service_config_id}`

Partial update service configuration `service_config_id` with the provided (partial) template.

![Partial updates](docs/images/partial_update_examples.png)

<details>
<summary>Request</summary>

```json
{
"configurations": {...},
"description": "Trader agent for omen prediction markets",
"env_variables": {...},
"hash": "bafybeibpseosblmaw6sk6zsnic2kfxfsijrnfluuhkwboyqhx7ma7zw2me",
"image": "https://operate.olas.network/_next/image?url=%2Fimages%2Fprediction-agent.png&w=3840&q=75",
"home_chain": "gnosis",
"name": "valory/trader_omen_gnosis",
"service_version": "v0.19.0"
}
```

</details>

<details>
<summary>Response</summary>

- If the update is successful, the response contains the updated service configuration:

```json
{
"chain_configs": {...},
"description": "Trader agent for omen prediction markets",
"env_variables": {...},
"hash": "bafybeidicxsruh3r4a2xarawzan6ocwyvpn3ofv42po5kxf7x6ck7kn22u",
"hash_history": {"1731487112": "bafybeidicxsruh3r4a2xarawzan6ocwyvpn3ofv42po5kxf7x6ck7kn22u", "1731490000": "bafybeibpseosblmaw6sk6zsnic2kfxfsijrnfluuhkwboyqhx7ma7zw2me"},
"home_chain": "gnosis",
"keys": [...],
"name": "valory/trader_omen_gnosis",
"service_config_id": "sc-85a7a12a-8c6b-46b8-919a-b8a3b8e3ad39",
"service_path": "/home/user/.operate/services/sc-85a7a12a-8c6b-46b8-919a-b8a3b8e3ad39/trader_omen_gnosis"
}

```

- If the update is not successful:

```json
{
"error": "Error message",
"traceback": "Traceback message"
}
```

</details>

---

### `POST /api/service/{service_config_id}/stop`

Stop service with service configuration `service_configuration_id`.
Expand Down
Binary file added docs/images/partial_update_examples.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ const useServiceDeployment = () => {
}

// Update the service if the hash is different
// TODO: "updateService" now uses PATCH instead of PUT,
// which means it updates only the provided values.
// Ideally we should consider rewriting this code
// so that only hash value is passed
if (!middlewareServiceResponse && service) {
if (service.hash !== serviceTemplate.hash) {
await ServicesService.updateService({
Expand Down
23 changes: 2 additions & 21 deletions frontend/service/Services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,27 +109,8 @@ const updateService = async ({
useMechMarketplace?: boolean;
}): Promise<MiddlewareServiceResponse> =>
fetch(`${BACKEND_URL_V2}/service/${serviceConfigId}`, {
method: 'PUT',
body: JSON.stringify({
...serviceTemplate,
deploy,
configurations: {
...serviceTemplate.configurations,
// overwrite defaults with chain-specific configurations
...Object.entries(serviceTemplate.configurations).reduce(
(acc, [middlewareChain, config]) => {
acc[middlewareChain] = {
...config,
rpc: CHAIN_CONFIG[asEvmChainId(middlewareChain)].rpc,
staking_program_id: stakingProgramId,
use_mech_marketplace: useMechMarketplace,
};
return acc;
},
{} as typeof serviceTemplate.configurations,
),
},
}),
method: 'PATCH',
body: JSON.stringify({ hash: serviceTemplate.hash }),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Don't pass the full template here! a PATCH Method will override all passed values here, including the stored environment variables on the backend. See the example here.

headers: { ...CONTENT_TYPE_JSON_UTF8 },
}).then((response) => {
if (response.ok) {
Expand Down
15 changes: 13 additions & 2 deletions operate/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ def pause_all_services_on_exit(signum: int, frame: t.Optional[FrameType]) -> Non
app.add_middleware(
CORSMiddleware,
allow_origins=["*"],
allow_methods=["GET", "POST", "PUT", "DELETE"],
allow_methods=["GET", "POST", "PUT", "PATCH", "DELETE"],
)

def with_retries(f: t.Callable) -> t.Callable:
Expand Down Expand Up @@ -756,6 +756,7 @@ def _fn() -> None:
)

@app.put("/api/v2/service/{service_config_id}")
@app.patch("/api/v2/service/{service_config_id}")
@with_retries
async def _update_service(request: Request) -> JSONResponse:
"""Update a service."""
Expand All @@ -765,18 +766,28 @@ async def _update_service(request: Request) -> JSONResponse:
service_config_id = request.path_params["service_config_id"]
manager = operate.service_manager()

print(service_config_id)
if not manager.exists(service_config_id=service_config_id):
return service_not_found_error(service_config_id=service_config_id)

template = await request.json()
allow_different_service_public_id = template.get(
"allow_different_service_public_id", False
)

if request.method == "PUT":
partial_update = False
else:
partial_update = True

logger.info(
f"_update_service {partial_update=} {allow_different_service_public_id=}"
)

output = manager.update(
service_config_id=service_config_id,
service_template=template,
allow_different_service_public_id=allow_different_service_public_id,
partial_update=partial_update,
)

return JSONResponse(content=output.json)
Expand Down
2 changes: 1 addition & 1 deletion operate/operate_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ class EnvVariableAttributes(TypedDict):
EnvVariables = t.Dict[str, EnvVariableAttributes]


class ServiceTemplate(TypedDict):
class ServiceTemplate(TypedDict, total=False):
"""Service template."""

name: str
Expand Down
40 changes: 37 additions & 3 deletions operate/services/manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -585,8 +585,37 @@ def _deploy_service_onchain_from_safe( # pylint: disable=too-many-statements,to
):
# TODO: This is possibly not a good idea: we are setting up a computed variable based on
# the value passed in the template.
db_path = service.path / "persistent_data/memeooorr.db"
cookies_path = service.path / "persistent_data/twikit_cookies.json"
db_path = (
service.path
/ "persistent_data"
/ service.env_variables["TWIKIT_USERNAME"]["value"]
/ "memeooorr.db"
)
cookies_path = (
service.path
/ "persistent_data"
/ service.env_variables["TWIKIT_USERNAME"]["value"]
/ "twikit_cookies.json"
)

# Patch: Move existing configurations to the new location
old_db_path = service.path / "persistent_data" / "memeooorr.db"
old_cookies_path = service.path / "persistent_data" / "twikit_cookies.json"

for old_path, new_path in [
(old_db_path, db_path),
(old_cookies_path, cookies_path),
]:
if old_path.exists():
self.logger.info(f"Moving {old_path} -> {new_path}")
new_path.parent.mkdir(parents=True, exist_ok=True)
try:
os.rename(old_path, new_path)
except OSError:
self.logger.info("Fallback to shutil.move")
shutil.move(str(old_path), str(new_path))
time.sleep(3)
# End patch

env_var_to_value.update(
{
Expand Down Expand Up @@ -1689,12 +1718,17 @@ def update(
service_config_id: str,
service_template: ServiceTemplate,
allow_different_service_public_id: bool = False,
partial_update: bool = True,
) -> Service:
"""Update a service."""

self.logger.info(f"Updating {service_config_id=}")
service = self.load(service_config_id=service_config_id)
service.update(service_template, allow_different_service_public_id)
service.update(
service_template=service_template,
allow_different_service_public_id=allow_different_service_public_id,
partial_update=partial_update,
)
return service

def update_all_matching(
Expand Down
74 changes: 46 additions & 28 deletions operate/services/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
SERVICE_CONFIG_VERSION = 4
SERVICE_CONFIG_PREFIX = "sc-"

DUMMY_MULTISIG = "0xm"
NON_EXISTENT_MULTISIG = "0xm"
NON_EXISTENT_TOKEN = -1

DEFAULT_TRADER_ENV_VARS = {
Expand Down Expand Up @@ -683,7 +683,7 @@ class Service(LocalResource):
_file = "config.json"

@classmethod
def migrate_format(cls, path: Path) -> bool:
def migrate_format(cls, path: Path) -> bool: # pylint: disable=too-many-statements
"""Migrate the JSON file format if needed."""

if not path.is_dir():
Expand Down Expand Up @@ -897,7 +897,7 @@ def new( # pylint: disable=too-many-locals
chain_data = OnChainData(
instances=[],
token=NON_EXISTENT_TOKEN,
multisig=DUMMY_MULTISIG,
multisig=NON_EXISTENT_MULTISIG,
staked=False,
on_chain_state=OnChainState.NON_EXISTENT,
user_params=OnChainUserParams.from_json(config), # type: ignore
Expand Down Expand Up @@ -981,47 +981,65 @@ def update(
self,
service_template: ServiceTemplate,
allow_different_service_public_id: bool = False,
partial_update: bool = False,
) -> None:
"""Update service."""

target_hash = service_template["hash"]
target_service_public_id = Service.get_service_public_id(target_hash, self.path)

if not allow_different_service_public_id and (
self.service_public_id() != target_service_public_id
):
raise ValueError(
f"Trying to update a service with a different public id: {self.service_public_id()=} {self.hash=} {target_service_public_id=} {target_hash=}."
target_hash = service_template.get("hash")
if target_hash:
target_service_public_id = Service.get_service_public_id(
target_hash, self.path
)

if not allow_different_service_public_id and (
self.service_public_id() != target_service_public_id
):
raise ValueError(
f"Trying to update a service with a different public id: {self.service_public_id()=} {self.hash=} {target_service_public_id=} {target_hash=}."
)

self.hash = service_template.get("hash", self.hash)

# hash_history - Only update if latest inserted hash is different
if self.hash_history[max(self.hash_history.keys())] != self.hash:
current_timestamp = int(time.time())
self.hash_history[current_timestamp] = self.hash

self.home_chain = service_template.get("home_chain", self.home_chain)
self.description = service_template.get("description", self.description)
self.name = service_template.get("name", self.name)

shutil.rmtree(self.service_path)
service_path = Path(
IPFSTool().download(
hash_id=service_template["hash"],
hash_id=self.hash,
target_dir=self.path,
)
)
self.service_path = service_path
self.name = service_template["name"]
self.hash = service_template["hash"]
self.description = service_template["description"]

# TODO temporarily disable update env variables - hotfix for Memeooorr
# self.env_variables = service_template["env_variables"]

# Only update hash_history if latest inserted hash is different
if self.hash_history[max(self.hash_history.keys())] != service_template["hash"]:
current_timestamp = int(time.time())
self.hash_history[current_timestamp] = service_template["hash"]

self.home_chain = service_template["home_chain"]
# env_variables
if partial_update:
for var, attrs in service_template.get("env_variables", {}).items():
self.env_variables.setdefault(var, {}).update(attrs)
else:
self.env_variables = service_template["env_variables"]

# chain_configs
# TODO support remove chains for non-partial updates
# TODO ensure all and only existing chains are passed for non-partial updates
ledger_configs = ServiceHelper(path=self.service_path).ledger_configs()
for chain, config in service_template["configurations"].items():
for chain, new_config in service_template.get("configurations", {}).items():
if chain in self.chain_configs:
# The template is providing a chain configuration that already
# exists in this service - update only the user parameters.
# This is to avoid losing on-chain data like safe, token, etc.
if partial_update:
config = self.chain_configs[chain].chain_data.user_params.json
config.update(new_config)
else:
config = new_config

self.chain_configs[
chain
].chain_data.user_params = OnChainUserParams.from_json(
Expand All @@ -1032,15 +1050,15 @@ def update(
# not currently exist in this service - copy all config as
# when creating a new service.
ledger_config = ledger_configs[chain]
ledger_config.rpc = config["rpc"]
ledger_config.rpc = new_config["rpc"]

chain_data = OnChainData(
instances=[],
token=NON_EXISTENT_TOKEN,
multisig=DUMMY_MULTISIG,
multisig=NON_EXISTENT_MULTISIG,
staked=False,
on_chain_state=OnChainState.NON_EXISTENT,
user_params=OnChainUserParams.from_json(config), # type: ignore
user_params=OnChainUserParams.from_json(new_config), # type: ignore
)

self.chain_configs[chain] = ChainConfig(
Expand Down
Loading
Loading