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.

Sign up for GitHub

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

Jump to bottom

Only one instance on GCP deployment and small tweaks #18

Merged
merged 6 commits into from
Feb 20, 2024
Merged

Only one instance on GCP deployment and small tweaks #18

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d0615e2
Fix retries on GCP deployment
kongzii Feb 19, 2024
dc0de77
Fix variable
kongzii Feb 19, 2024
10d8625
Fix type
kongzii Feb 19, 2024
9169eea
lint
kongzii Feb 19, 2024
6cf89e4
remove todo
kongzii Feb 19, 2024
53b9c19
custom gcp name support
kongzii Feb 19, 2024
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
25 changes: 20 additions & 5 deletions examples/cloud_deployment/gcp/deploy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
import getpass
import typer

from prediction_market_agent_tooling.deploy.agent_example import DeployableCoinFlipAgent
from prediction_market_agent_tooling.deploy.agent_example import (
DeployableAgent,
DeployableCoinFlipAgent,
DeployableAlwaysRaiseAgent,
)
from prediction_market_agent_tooling.markets.markets import MarketType

if __name__ == "__main__":
agent = DeployableCoinFlipAgent()

def main(
agent_name: str, cron_schedule: str = "0 */2 * * *", branch: str = "main"
) -> None:
agent: DeployableAgent = {
"coin_flip": DeployableCoinFlipAgent,
"always_raise": DeployableAlwaysRaiseAgent,
}[agent_name]()
Copy link

Choose a reason for hiding this comment

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

The dynamic selection of agents based on the agent_name parameter is a significant improvement for deployment flexibility. However, consider validating the agent_name input against the available agent classes to prevent runtime errors from invalid inputs.

+ if agent_name not in ["coin_flip", "always_raise"]:
+     raise ValueError(f"Invalid agent_name: {agent_name}. Available options: 'coin_flip', 'always_raise'")
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
def main(
agent_name: str, cron_schedule: str = "0 */2 * * *", branch: str = "main"
) -> None:
agent: DeployableAgent = {
"coin_flip": DeployableCoinFlipAgent,
"always_raise": DeployableAlwaysRaiseAgent,
}[agent_name]()
def main(
agent_name: str, cron_schedule: str = "0 */2 * * *", branch: str = "main"
) -> None:
if agent_name not in ["coin_flip", "always_raise"]:
raise ValueError(f"Invalid agent_name: {agent_name}. Available options: 'coin_flip', 'always_raise'")
agent: DeployableAgent = {
"coin_flip": DeployableCoinFlipAgent,
"always_raise": DeployableAlwaysRaiseAgent,
}[agent_name]()

agent.deploy_gcp(
# TODO: Switch to main.
repository="git+https://github.com/gnosis/prediction-market-agent-tooling.git@peter/refactor-deployment",
repository=f"git+https://github.com/gnosis/prediction-market-agent-tooling.git@{branch}",
market_type=MarketType.MANIFOLD,
labels={
"owner": getpass.getuser()
Expand All @@ -18,5 +29,9 @@
"MANIFOLD_API_KEY": f"JUNG_PERSONAL_GMAIL_MANIFOLD_API_KEY:latest"
}, # Must be in the format "env_var_in_container => secret_name:version", you can create secrets using `gcloud secrets create --labels owner=<your-name> <secret-name>` command.
memory=256,
cron_schedule="0 */2 * * *",
cron_schedule=cron_schedule,
)


if __name__ == "__main__":
typer.run(main)
9 changes: 6 additions & 3 deletions prediction_market_agent_tooling/deploy/agent_example.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@

class DeployableCoinFlipAgent(DeployableAgent):
def pick_markets(self, markets: list[AgentMarket]) -> list[AgentMarket]:
if len(markets) > 1:
return random.sample(markets, 1)
return markets
return random.sample(markets, 1)

def answer_binary_market(self, market: AgentMarket) -> bool:
return random.choice([True, False])


class DeployableAlwaysRaiseAgent(DeployableAgent):
def answer_binary_market(self, market: AgentMarket) -> bool:
raise RuntimeError("I always raise!")
9 changes: 9 additions & 0 deletions prediction_market_agent_tooling/deploy/gcp/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ def gcloud_deploy_cmd(
env_vars: dict[str, str] | None,
secrets: dict[str, str] | None,
memory: int, # in MB
timeout: int = 180,
retry_on_failure: bool = False,
) -> str:
cmd = (
f"gcloud functions deploy {gcp_function_name} "
Expand All @@ -26,7 +28,14 @@ def gcloud_deploy_cmd(
f"--entry-point {entry_point} "
f"--memory {memory}MB "
f"--no-allow-unauthenticated "
f"--timeout {timeout}s "
# Explicitly set no concurrency, min instances to 0 (agent is executed only once in a while) and max instances to 1 (parallel agents aren't allowed).
"--concurrency 1 "
"--min-instances 0 "
"--max-instances 1 "
)
if retry_on_failure:
cmd += "--retry "
Comment on lines +31 to +38
Copy link

Choose a reason for hiding this comment

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

The implementation of the timeout and retry_on_failure parameters in the deployment command is correct. However, consider adding error handling or logging to provide feedback when a deployment fails despite retries. This can help in diagnosing deployment issues more efficiently.

+ if retry_on_failure:
+     cmd += "--retry "
+     print("Note: Deployment will retry on failure.")
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
f"--timeout {timeout}s "
# Explicitly set no concurrency, min instances to 0 (agent is executed only once in a while) and max instances to 1 (parallel agents aren't allowed).
"--concurrency 1 "
"--min-instances 0 "
"--max-instances 1 "
)
if retry_on_failure:
cmd += "--retry "
f"--timeout {timeout}s "
# Explicitly set no concurrency, min instances to 0 (agent is executed only once in a while) and max instances to 1 (parallel agents aren't allowed).
"--concurrency 1 "
"--min-instances 0 "
"--max-instances 1 "
)
if retry_on_failure:
cmd += "--retry "
print("Note: Deployment will retry on failure.")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what happened before with OpenAI-powered agents, this retry is disabled by default. I tried to deploy agents that always fail with changes from this PR and without, and both are executed just once per 10 min (as scheduled).

I think this PR introduces changes that are worth merging, but it doesn't solve the retries problem. I'll deploy the openai-powered agents tomorrow and monitor it...

Screenshot by Dropbox Capture

if labels:
for k, v in labels.items():
cmd += f"--update-labels {k}={v} "
Expand Down
Loading