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 12 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
56 changes: 56 additions & 0 deletions api.md
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,62 @@ Update service configuration `service_config_id` with the provided template.

---

### `PUSH /api/v2/service/{service_config_id}`
jmoreira-valory marked this conversation as resolved.
Show resolved Hide resolved

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

<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
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
2 changes: 1 addition & 1 deletion frontend/service/Services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const updateService = async ({
useMechMarketplace?: boolean;
}): Promise<MiddlewareServiceResponse> =>
fetch(`${BACKEND_URL_V2}/service/${serviceConfigId}`, {
method: 'PUT',
method: 'PATCH',
body: JSON.stringify({
...serviceTemplate,
deploy,
Expand Down
9 changes: 8 additions & 1 deletion operate/cli.py
Original file line number Diff line number Diff line change
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,24 @@ 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

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