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

Only one instance on GCP deployment and small tweaks #18

Merged
merged 6 commits into from
Feb 20, 2024
Merged
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
31 changes: 25 additions & 6 deletions examples/cloud_deployment/gcp/deploy.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,27 @@
import getpass

from prediction_market_agent_tooling.deploy.agent_example import DeployableCoinFlipAgent
import typer

from prediction_market_agent_tooling.deploy.agent_example import (
DeployableAgent,
DeployableAlwaysRaiseAgent,
DeployableCoinFlipAgent,
)
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",
custom_gcp_fname: str | None = None,
) -> None:
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 +32,10 @@
"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,
gcp_fname=custom_gcp_fname,
)


if __name__ == "__main__":
typer.run(main)
3 changes: 2 additions & 1 deletion prediction_market_agent_tooling/deploy/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def deploy_gcp(
env_vars: dict[str, str] | None = None,
secrets: dict[str, str] | None = None,
cron_schedule: str | None = None,
gcp_fname: str | None = None,
) -> None:
path_to_agent_file = os.path.relpath(inspect.getfile(self.__class__))

Expand All @@ -85,7 +86,7 @@ def main(request) -> str:
return "Success"
"""

gcp_fname = self.get_gcloud_fname(market_type)
gcp_fname = gcp_fname or self.get_gcloud_fname(market_type)

with tempfile.NamedTemporaryFile(mode="w", suffix=".py") as f:
f.write(entrypoint_template)
Comment on lines 86 to 92
Copy link

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [74-104]

The deploy_gcp method demonstrates a comprehensive approach to deploying agents to GCP, including dynamic entrypoint generation, deployment parameter handling, and error checking. However, it's important to ensure that the deploy_to_gcp and related functions are robustly implemented to handle the dynamic nature of the entrypoint generation and deployment parameters. Additionally, consider adding more detailed error messages to help diagnose deployment failures more effectively.

Consider enhancing error messages in the deployment process to provide more context on failures, which could aid in troubleshooting.

Expand Down
9 changes: 6 additions & 3 deletions prediction_market_agent_tooling/deploy/agent_example.py
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
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
2 changes: 1 addition & 1 deletion prediction_market_agent_tooling/markets/data_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ class ManifoldMarket(BaseModel):
isResolved: bool
resolution: t.Optional[str] = None
resolutionTime: t.Optional[datetime] = None
lastBetTime: datetime
lastBetTime: t.Optional[datetime] = None
lastCommentTime: t.Optional[datetime] = None
lastUpdatedTime: datetime
mechanism: str
Expand Down
Loading