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

Fix/groups sync #684

Merged
merged 7 commits into from
Oct 18, 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
62 changes: 43 additions & 19 deletions app/integrations/google_workspace/google_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
DEFAULT_DELEGATED_ADMIN_EMAIL,
DEFAULT_GOOGLE_WORKSPACE_CUSTOMER_ID,
)
from integrations.utils.api import convert_string_to_camel_case
from integrations.utils.api import convert_string_to_camel_case, retry_request
from utils import filters

logger = getLogger(__name__)
Expand Down Expand Up @@ -209,37 +209,61 @@ def list_groups_with_members(

groups_with_members = []
for group in filtered_groups:
error_occured = False
logger.info(f"Getting members for group: {group['email']}")
try:
members = list_group_members(
group["email"], fields="members(email, role, type, status)"
members = retry_request(
list_group_members,
group["email"],
max_attempts=3,
delay=1,
fields="members(email, role, type, status)",
)
except Exception as e:
logger.warning(f"Error getting members for group {group['email']}: {e}")
continue

for member in members:
user_details = {}
try:
logger.info(f"Getting user details for member: {member['email']}")
user_details = get_user(member["email"], fields="name, primaryEmail")
except Exception as e:
logger.warning(
f"Error getting user details for member {member['email']}: {e}"
)
error_occured = True
if not tolerate_errors:
break
if user_details:
member.update(user_details)
if members and (not error_occured or tolerate_errors):
members = get_members_details(members, tolerate_errors)
if members:
group.update({"members": members})
groups_with_members.append(group)

return groups_with_members


def get_members_details(members: list[dict], tolerate_errors=False):
"""Get user details for a list of members.

Args:
members (list): A list of member objects.
tolerate_errors (bool): Whether to tolerate errors when getting user details.

Returns:"""

error_occured = False
for member in members:
user_details = {}
try:
logger.info(f"Getting user details for member: {member['email']}")
user_details = retry_request(
get_user,
member["email"],
max_attempts=3,
delay=1,
fields="name, primaryEmail",
)
except Exception as e:
logger.warning(
f"Error getting user details for member {member['email']}: {e}"
)
error_occured = True
if not tolerate_errors:
break
if user_details:
member.update(user_details)

return members if not error_occured or tolerate_errors else []


def convert_google_groups_members_to_dataframe(groups):
"""Converts a list of Google groups with members to a DataFrame.

Expand Down
6 changes: 5 additions & 1 deletion app/integrations/google_workspace/google_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,22 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
logging.error(
f"An HTTP error occurred in function '{func.__module__}:{func.__name__}': {e}"
)
raise e
except ValueError as e:
logging.error(
f"A ValueError occurred in function '{func.__module__}:{func.__name__}': {e}"
)
raise e
except RefreshError as e:
logging.error(
f"A RefreshError occurred in function '{func.__module__}:{func.__name__}': {e}"
)
raise e
except Error as e:
logging.error(
f"An error occurred in function '{func.__module__}:{func.__name__}': {e}"
)
raise e
except Exception as e: # Catch-all for any other types of exceptions
message = str(e)
func_name = func.__name__
Expand All @@ -131,7 +135,7 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
logging.error(
f"An unexpected error occurred in function '{func.__module__}:{func.__name__}': {e}"
)
return None
raise e

return wrapper

Expand Down
32 changes: 32 additions & 0 deletions app/integrations/utils/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import re
import string
import random
import time
import logging


def convert_string_to_camel_case(snake_str):
Expand Down Expand Up @@ -85,3 +87,33 @@ def generate_segment():
unique_id = "-".join(segments)

return unique_id


def retry_request(
func,
*args,
max_attempts=3,
delay=1,
**kwargs,
):
"""Retry a function up to a maximum number of attempts with a delay between each attempt.

Args:
func (function): The function to call.
max_attempts (int): The maximum number of attempts to make.
delay (int): The delay between each attempt in seconds.
*args: Positional arguments to pass to the function.
**kwargs: Keyword arguments to pass to the function.

Returns:
Any: The result of the function call.
"""
for i in range(max_attempts):
try:
return func(*args, **kwargs)
except Exception as e:
if i == max_attempts - 1:
logging.warning(f"Error after {max_attempts} attempts: {e}")
raise e
time.sleep(delay)
continue
11 changes: 6 additions & 5 deletions app/tests/integrations/google_workspace/test_google_calendar.py
Original file line number Diff line number Diff line change
Expand Up @@ -470,11 +470,12 @@ def test_insert_event_api_call_error(
emails = ["[email protected]", "[email protected]"]
title = "Test Event"
document_id = "test_document_id"
google_calendar.insert_event(start, end, emails, title, document_id)
assert (
"An unexpected error occurred in function 'integrations.google_workspace.google_calendar:insert_event': API call error"
in caplog.text
)
with pytest.raises(Exception):
google_calendar.insert_event(start, end, emails, title, document_id)
assert (
"An unexpected error occurred in function 'integrations.google_workspace.google_calendar:insert_event': API call error"
in caplog.text
)
assert not mock_convert_string_to_camel_case.called
assert mock_os_environ_get.called
assert not mock_handle_errors.called
Expand Down
97 changes: 79 additions & 18 deletions app/tests/integrations/google_workspace/test_google_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -395,13 +395,11 @@ def test_list_groups_with_members_filtered(
assert mock_get_user.call_count == 2


@patch("integrations.google_workspace.google_directory.retry_request")
@patch("integrations.google_workspace.google_directory.list_groups")
@patch("integrations.google_workspace.google_directory.list_group_members")
@patch("integrations.google_workspace.google_directory.get_user")
def test_list_groups_with_members_error_in_list_group_members(
mock_get_user,
mock_list_group_members,
mock_list_groups,
mock_retry_request,
google_groups,
google_group_members,
google_users,
Expand All @@ -412,8 +410,12 @@ def test_list_groups_with_members_error_in_list_group_members(
users = google_users(2)

mock_list_groups.return_value = groups
mock_list_group_members.side_effect = group_members
mock_get_user.side_effect = users
mock_retry_request.side_effect = [
group_members[0],
group_members[1],
users[0],
users[1],
]

# Only the second group should be processed
expected_groups_with_users = [groups[1]]
Expand All @@ -424,13 +426,13 @@ def test_list_groups_with_members_error_in_list_group_members(
assert google_directory.list_groups_with_members() == expected_groups_with_users


@patch("integrations.google_workspace.google_directory.get_members_details")
@patch("integrations.google_workspace.google_directory.retry_request")
@patch("integrations.google_workspace.google_directory.list_groups")
@patch("integrations.google_workspace.google_directory.list_group_members")
@patch("integrations.google_workspace.google_directory.get_user")
def test_list_groups_with_members_error_in_get_user(
mock_get_user,
mock_list_group_members,
mock_list_groups,
mock_retry_request,
mock_get_members_details,
google_groups,
google_group_members,
google_users,
Expand All @@ -445,8 +447,14 @@ def test_list_groups_with_members_error_in_get_user(
]

mock_list_groups.return_value = groups
mock_list_group_members.side_effect = group_members
mock_get_user.side_effect = users
mock_retry_request.side_effect = [
group_members[0],
group_members[1],
]
mock_get_members_details.side_effect = [
[],
group_members[1],
]

# Only the second group should be processed
expected_groups_with_users = [groups[1]]
Expand All @@ -457,13 +465,11 @@ def test_list_groups_with_members_error_in_get_user(
assert google_directory.list_groups_with_members() == expected_groups_with_users


@patch("integrations.google_workspace.google_directory.retry_request")
@patch("integrations.google_workspace.google_directory.list_groups")
@patch("integrations.google_workspace.google_directory.list_group_members")
@patch("integrations.google_workspace.google_directory.get_user")
def test_list_groups_with_members_tolerate_errors(
mock_get_user,
mock_list_group_members,
mock_list_groups,
mock_retry_request,
google_groups_w_users,
):

Expand Down Expand Up @@ -501,8 +507,14 @@ def test_list_groups_with_members_tolerate_errors(
]

mock_list_groups.return_value = groups
mock_list_group_members.side_effect = group_members
mock_get_user.side_effect = users
mock_retry_request.side_effect = [
group_members[0],
users[0],
users[1],
group_members[1],
users[2],
users[3],
]

# Expected result should include both groups, with the second group having one member
expected_groups_with_users = [
Expand Down Expand Up @@ -617,3 +629,52 @@ def test_list_groups_with_members_filtered_dataframe(
"member_given_name",
"member_family_name",
}


@patch("integrations.google_workspace.google_directory.retry_request")
def test_get_members_details_breaks_on_error(mock_retry_request):
members = [
{"email": "email1", "role": "MEMBER", "type": "USER", "status": "ACTIVE"},
{"email": "email2", "role": "MEMBER", "type": "USER", "status": "ACTIVE"},
]

users = [
Exception("Error fetching user details"),
{"name": "user2", "primaryEmail": "email2"},
]

mock_retry_request.side_effect = [users[0], users[1]]

result = google_directory.get_members_details(members)

assert result == []


@patch("integrations.google_workspace.google_directory.retry_request")
def test_get_members_details_continues_on_tolerate_errors(mock_retry_request):
members = [
{"email": "email1", "role": "MEMBER", "type": "USER", "status": "ACTIVE"},
{"email": "email2", "role": "MEMBER", "type": "USER", "status": "ACTIVE"},
]

users = [
Exception("Error fetching user details"),
{"name": "user2", "primaryEmail": "email2"},
]

mock_retry_request.side_effect = [users[0], users[1]]

result = google_directory.get_members_details(members, tolerate_errors=True)

expected_result = [
members[0],
{
"email": "email2",
"role": "MEMBER",
"type": "USER",
"status": "ACTIVE",
"name": "user2",
"primaryEmail": "email2",
},
]
assert result == expected_result
7 changes: 4 additions & 3 deletions app/tests/integrations/google_workspace/test_google_drive.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Unit tests for google_drive module."""

from unittest.mock import patch, call

import pytest
from integrations.google_workspace import google_drive


Expand Down Expand Up @@ -152,9 +152,10 @@ def test_create_file_with_invalid_type_raises_value_error(
execute_google_api_call_mock.side_effect = ValueError(
"Invalid file_type: invalid_file_type"
)
result = google_drive.create_file("name", "folder", "invalid_file_type")
with pytest.raises(ValueError):
result = google_drive.create_file("name", "folder", "invalid_file_type")
assert result is None

assert result is None
mocked_logging_error.assert_called_once_with(
"A ValueError occurred in function 'integrations.google_workspace.google_drive:create_file': Invalid file_type: invalid_file_type"
)
Expand Down
Loading
Loading