Skip to content

Commit

Permalink
Retry GADs requests on failure and update packages (#984)
Browse files Browse the repository at this point in the history 
* Update messages displayed to the user

* Add retry with backoff

* Add check for clients approval

* Update packages

* Add exponential backoff
  • Loading branch information
@rjambrecic
rjambrecic authored Oct 14, 2024
1 parent eaea27a commit c09fd85
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 38 deletions.
83 changes: 58 additions & 25 deletions captn/captn_agents/backend/tools/_gbb_page_feed_team_tools.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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(
Expand All @@ -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
)
Expand All @@ -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


Expand All @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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 = ""
Expand All @@ -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


Expand All @@ -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."

Expand All @@ -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()
Expand Down
11 changes: 10 additions & 1 deletion captn/google_ads/client.py
View file
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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]
10 changes: 5 additions & 5 deletions pyproject.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions tests/ci/captn/captn_agents/backend/teams/test_gbb_page_feed_team.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:",
]
Expand Down
47 changes: 42 additions & 5 deletions tests/ci/captn/captn_agents/backend/tools/test_gbb_page_feed_team_tools.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
),
(
[
Expand All @@ -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""",
),
(
[
Expand All @@ -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""",
),
],
)
Expand Down Expand Up @@ -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

0 comments on commit c09fd85

Please sign in to comment.