From c09fd8589e709e659540c57979cbcbae79d72b74 Mon Sep 17 00:00:00 2001 From: rjambrecic <32619626+rjambrecic@users.noreply.github.com> Date: Mon, 14 Oct 2024 11:55:37 +0200 Subject: [PATCH] Retry GADs requests on failure and update packages (#984) * Update messages displayed to the user * Add retry with backoff * Add check for clients approval * Update packages * Add exponential backoff --- .../tools/_gbb_page_feed_team_tools.py | 83 +++++++++++++------ captn/google_ads/client.py | 11 ++- pyproject.toml | 10 +-- .../backend/teams/test_gbb_page_feed_team.py | 4 +- .../tools/test_gbb_page_feed_team_tools.py | 47 +++++++++-- 5 files changed, 117 insertions(+), 38 deletions(-) diff --git a/captn/captn_agents/backend/tools/_gbb_page_feed_team_tools.py b/captn/captn_agents/backend/tools/_gbb_page_feed_team_tools.py index 467ec1a5..dfd31142 100644 --- a/captn/captn_agents/backend/tools/_gbb_page_feed_team_tools.py +++ b/captn/captn_agents/backend/tools/_gbb_page_feed_team_tools.py @@ -10,7 +10,9 @@ from google_ads.model import AddPageFeedItems # , RemoveResource from ....google_ads.client import ( + check_for_client_approval, execute_query, + google_ads_api_call, google_ads_create_update, # noqa google_ads_post_or_get, ) @@ -322,18 +324,25 @@ def _add_missing_page_urls( asset_set_resource_name=page_feed_asset_set["resourceName"], urls_and_labels=url_and_labels, ) - response = google_ads_post_or_get( - user_id=user_id, - conv_id=conv_id, - model=add_model, - recommended_modifications_and_answer_list=[], - already_checked_clients_approval=True, - endpoint="/add-items-to-page-feed", - ) + + try: + response = google_ads_api_call( + function=google_ads_post_or_get, # type: ignore[arg-type] + kwargs={ + "user_id": user_id, + "conv_id": conv_id, + "model": add_model, + "recommended_modifications_and_answer_list": [], + "already_checked_clients_approval": True, + "endpoint": "/add-items-to-page-feed", + }, + ) + except Exception as e: + return f"Failed to add page feed items:\n{url_and_labels}\n\n{str(e)}\n\n" if isinstance(response, dict): raise ValueError(response) urls_to_string = "\n".join(url_and_labels.keys()) - return f"Added page feed items:\n{urls_to_string}\n" + return f"Added page feed items:\n{urls_to_string}\n\n" def _remove_extra_page_urls( @@ -355,21 +364,26 @@ def _remove_extra_page_urls( # resource_id=id, # resource_type="asset_set_asset", # ) - - # response = google_ads_create_update( - # user_id=user_id, - # conv_id=conv_id, - # recommended_modifications_and_answer_list=[], - # already_checked_clients_approval=True, - # ad=remove_model, - # login_customer_id=login_customer_id, - # endpoint="/remove-google-ads-resource", - # ) + # try: + # response = google_ads_api_call( + # function=google_ads_create_update, # type: ignore[arg-type] + # kwargs={ + # "user_id": user_id, + # "conv_id": conv_id, + # "recommended_modifications_and_answer_list": [], + # "already_checked_clients_approval": True, + # "ad": remove_model, + # "login_customer_id": login_customer_id, + # "endpoint": "/remove-google-ads-resource", + # }, + # ) + # except Exception as e: + # return f"Failed to remove page feed item with id {id} - {row[1]['Page URL']}:\n{str(e)}\n\n" response = "REMOVE THIS LINE ONCE THE ABOVE CODE IS UNCOMMENTED" if isinstance(response, dict): msg = f"Failed to remove page feed item with id {id} - {row[1]['Page URL']}:\n" - return_value += msg + str(response) + return_value += msg + str(response) + "\n\n" iostream.print( colored(f"[{get_time()}] " + msg + str(response), "red"), flush=True ) @@ -378,6 +392,7 @@ def _remove_extra_page_urls( return_value += msg + "\n" iostream.print(colored(f"[{get_time()}] " + msg, "green"), flush=True) + return_value += "\n" return return_value @@ -396,12 +411,12 @@ def _sync_page_feed_asset_set( page_feeds_and_accounts_templ_df["Name Page Feed"] == page_feed_asset_set_name ] if page_feed_rows.empty: - msg = f"Skipping page feed '{page_feed_asset_set_name}' (not found in the page feed template).\n" + msg = f"Skipping page feed '**{page_feed_asset_set_name}**' (not found in the page feed template).\n\n" iostream.print(colored(f"[{get_time()}] " + msg, "yellow"), flush=True) return msg elif page_feed_rows["Customer Id"].nunique() > 1: - msg = f"Page feed template has multiple values for the same page feed '{page_feed_asset_set_name}'!\n" + msg = f"Page feed template has multiple values for the same page feed '**{page_feed_asset_set_name}**'!\n\n" iostream.print(colored(f"[{get_time()}] " + msg, "red"), flush=True) return msg @@ -416,7 +431,7 @@ def _sync_page_feed_asset_set( ] if page_feed_rows.empty: - msg = f"No page feed data found for page feed '{page_feed_asset_set_name}'\n" + msg = f"No page feed data found for page feed '**{page_feed_asset_set_name}**'\n\n" iostream.print(colored(f"[{get_time()}] " + msg, "yellow"), flush=True) return msg @@ -445,11 +460,11 @@ def _sync_page_feed_asset_set( ] if missing_page_urls.empty and extra_page_urls.empty: - msg = f"No changes needed for page feed '{page_feed_asset_set_name}'\n" + msg = f"No changes needed for page feed '**{page_feed_asset_set_name}**'\n\n" iostream.print(colored(f"[{get_time()}] " + msg, "green"), flush=True) return msg - return_value = f"Page feed '{page_feed_asset_set_name}' changes:\n" + return_value = f"Page feed '**{page_feed_asset_set_name}**' changes:\n" iostream.print( colored(f"[{get_time()}] " + return_value.strip(), "green"), flush=True ) @@ -487,6 +502,7 @@ def _sync_page_feed_asset_sets( page_feeds_and_accounts_templ_df: pd.DataFrame, page_feeds_df: pd.DataFrame, page_feed_asset_sets: Dict[str, Dict[str, str]], + context: PageFeedTeamContext, ) -> str: iostream = IOStream.get_default() return_value = "" @@ -503,6 +519,12 @@ def _sync_page_feed_asset_sets( iostream=iostream, ) + return_value = reply_to_client( + message=return_value, + completed=False, + context=context, + ) + return return_value @@ -511,6 +533,16 @@ def update_page_feeds( login_customer_id: Annotated[str, "Login Customer Id (Manager Account)"], context: PageFeedTeamContext, ) -> str: + error_msg = check_for_client_approval( + modification_function_parameters={ + "customer_id": customer_id, + "login_customer_id": login_customer_id, + }, + recommended_modifications_and_answer_list=context.recommended_modifications_and_answer_list, + ) + if error_msg: + raise ValueError(error_msg) + if context.page_feeds_and_accounts_templ_df is None: return f"Please (re)validate the page feed data first by running the '{get_and_validate_page_feed_data.__name__}' function." @@ -532,6 +564,7 @@ def update_page_feeds( page_feeds_and_accounts_templ_df=context.page_feeds_and_accounts_templ_df, page_feeds_df=context.page_feeds_df, page_feed_asset_sets=page_feed_asset_sets, + context=context, ) except Exception as e: traceback.print_stack() diff --git a/captn/google_ads/client.py b/captn/google_ads/client.py index 8b131786..8f6cfeed 100644 --- a/captn/google_ads/client.py +++ b/captn/google_ads/client.py @@ -1,10 +1,11 @@ import json from os import environ -from typing import Any, Dict, List, Literal, Optional, Tuple, Union +from typing import Any, Callable, Dict, List, Literal, Optional, Tuple, Union from pydantic import BaseModel from requests import get as requests_get from requests import post as requests_post +from tenacity import retry, stop_after_attempt, wait_exponential BASE_URL = environ.get("CAPTN_BACKEND_URL", "http://localhost:9000") ALREADY_AUTHENTICATED = "User is already authenticated" @@ -388,3 +389,11 @@ def google_ads_post_or_get( response_dict: Union[Dict[str, Any], str] = response.json() return response_dict + + +@retry(stop=stop_after_attempt(3), wait=wait_exponential(min=4, max=15)) +def google_ads_api_call( + function: Callable[[Any], Union[Dict[str, Any], str]], + **kwargs: Any, +) -> Union[Dict[str, Any], str]: + return function(**kwargs) # type: ignore[call-arg] diff --git a/pyproject.toml b/pyproject.toml index e419d96e..84f9a164 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -62,12 +62,12 @@ lint = [ "pyupgrade-directories", "bandit==1.7.10", "semgrep==1.78.0", - "pre-commit==4.0.0", + "pre-commit==4.0.1", "detect-secrets==1.5.0", ] test-core = [ - "coverage[toml]==7.6.1", + "coverage[toml]==7.6.3", "pytest==8.3.3", "pytest-asyncio>=0.23.6", "dirty-equals==0.8.0", @@ -94,7 +94,7 @@ agents = [ "prisma==0.13.1", "google-ads==25.0.0", "httpx==0.27.0", - "uvicorn==0.31.0", + "uvicorn==0.31.1", "python-dotenv==1.0.1", "pyautogen[websurfer,websockets,anthropic,together]==0.2.35", "pandas>=2.1", @@ -108,9 +108,9 @@ agents = [ "opentelemetry-exporter-otlp==1.27.0", "openpyxl==3.1.5", "aiofiles==24.1.0", - "fastapi==0.114.2", + "fastapi==0.115.2", "asyncer==0.0.8", - "fastagency[openapi]==0.2.1", + "fastagency[openapi]==0.2.5", "markdownify==0.13.1", "python-multipart==0.0.12", "flaml==2.2.0", diff --git a/tests/ci/captn/captn_agents/backend/teams/test_gbb_page_feed_team.py b/tests/ci/captn/captn_agents/backend/teams/test_gbb_page_feed_team.py index 0359e23f..fc4788f8 100644 --- a/tests/ci/captn/captn_agents/backend/teams/test_gbb_page_feed_team.py +++ b/tests/ci/captn/captn_agents/backend/teams/test_gbb_page_feed_team.py @@ -68,8 +68,8 @@ def test_page_feed_real_fastapi_team_end2end( ) expected_messages = [ - "Page feed 'fastagency-reference' changes:", - "Page feed 'fastagency-tutorial' changes:", + "Page feed '**fastagency-reference**' changes:", + "Page feed '**fastagency-tutorial**' changes:", "Added page feed items:", "The following page feed items should be removed by you manually:", ] diff --git a/tests/ci/captn/captn_agents/backend/tools/test_gbb_page_feed_team_tools.py b/tests/ci/captn/captn_agents/backend/tools/test_gbb_page_feed_team_tools.py index 559e1d51..0f1a192b 100644 --- a/tests/ci/captn/captn_agents/backend/tools/test_gbb_page_feed_team_tools.py +++ b/tests/ci/captn/captn_agents/backend/tools/test_gbb_page_feed_team_tools.py @@ -10,6 +10,7 @@ from captn.captn_agents.backend.config import Config from captn.captn_agents.backend.tools._gbb_page_feed_team_tools import ( PageFeedTeamContext, + _add_missing_page_urls, _get_page_feed_asset_sets, _get_page_feed_items, _get_relevant_page_feeds_and_accounts, @@ -385,7 +386,7 @@ def test_get_page_feed_asset_sets( ], } ), - "No changes needed for page feed 'fastagency-reference'\n", + "No changes needed for page feed '**fastagency-reference**'\n\n", ), ( [ @@ -407,9 +408,9 @@ def test_get_page_feed_asset_sets( ], } ), - """Page feed 'fastagency-reference' changes: + """Page feed '**fastagency-reference**' changes: The following page feed items should be removed by you manually: -- https://getbybus.com/it/bus-zagreb-to-split\n""", +- https://getbybus.com/it/bus-zagreb-to-split\n\n""", ), ( [ @@ -430,9 +431,9 @@ def test_get_page_feed_asset_sets( ], } ), - """Page feed 'fastagency-reference' changes: + """Page feed '**fastagency-reference**' changes: Added page feed items: -https://getbybus.com/hr/bus-zagreb-to-karlovac\n""", +https://getbybus.com/hr/bus-zagreb-to-karlovac\n\n""", ), ], ) @@ -520,3 +521,39 @@ def test_get_page_feed_items(self) -> None: assert page_urls_and_labels_df.sort_values("Page URL").equals( expected_page_urls_and_labels_df.sort_values("Page URL") ) + + def test_add_missing_page_urls(self) -> None: + with unittest.mock.patch( + "captn.captn_agents.backend.tools._gbb_page_feed_team_tools.google_ads_post_or_get", + ) as mock_google_ads_post_or_get: + mock_google_ads_post_or_get.side_effect = [ + ValueError("Error1"), + ValueError("Error2"), + "Created an asset set asset link", + ] + + result = _add_missing_page_urls( + user_id=-1, + conv_id=-1, + customer_id="1111", + login_customer_id="1111", + page_feed_asset_set={ + "resourceName": "customers/1111/assetSets/8783430659", + "id": "8783430659", + }, + missing_page_urls=pd.DataFrame( + { + "Page URL": [ + "https://fastagency.ai/latest/api/fastagency/FunctionCallExecution", + "https://fastagency.ai/latest/api/fastagency/FastAgency", + ], + "Custom Label": [None, None], + } + ), + ) + expected = """Added page feed items: +https://fastagency.ai/latest/api/fastagency/FunctionCallExecution +https://fastagency.ai/latest/api/fastagency/FastAgency\n\n""" + assert result == expected, result + + assert mock_google_ads_post_or_get.call_count == 3