Skip to content

Commit

Permalink
Link tasks to each TODO in download_logs.py (#993)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #993

# This stack:
* Adding tests for download_logs/
# This diff:
* Link task_ids to each TODO

Reviewed By: marksliva

Differential Revision: D36863598

fbshipit-source-id: 98ec94c4d5bc4f3d858da6c67e42d60693dda0c9
  • Loading branch information
Logan Gore authored and facebook-github-bot committed Jun 2, 2022
1 parent 6af6cc8 commit 0706c21
Showing 1 changed file with 15 additions and 15 deletions.
30 changes: 15 additions & 15 deletions fbpcs/infra/logging_service/download_logs/download_logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def get_cloudwatch_logs(
f"Unexpected error occured in fetching the log event log group {log_group_name} and log stream {log_stream_name}\n"
f"{error}\n"
)
# TODO: Raise more specific exception
# TODO T122315363: Raise more specific exception
raise Exception(f"{error_message}")

return messages
Expand All @@ -131,7 +131,7 @@ def create_s3_folder(self, bucket_name: str, folder_name: str) -> None:
error_message = (
f"Failed to create folder {folder_name} in S3 bucket {bucket_name}\n"
)
# TODO: Raise more specific exception
# TODO T122315363: Raise more specific exception
raise Exception(f"{error_message}")

def ensure_folder_exists(self, bucket_name: str, folder_name: str) -> bool:
Expand Down Expand Up @@ -184,7 +184,7 @@ def get_s3_folder_contents(
error_message = f"Couldn't find folder. Please check if S3 bucket name {bucket_name} and folder name {folder_name} are correct"
if error.response.get("Error", {}).get("Code") == "NoSuchBucket":
error_message = f"Couldn't find folder {folder_name} in S3 bucket {bucket_name}\n{error}"
# TODO: Raise more specific exception
# TODO T122315363: Raise more specific exception
raise Exception({error_message})

return response
Expand Down Expand Up @@ -214,7 +214,7 @@ def download_logs(
folder_name=f"{self.S3_LOGGING_FOLDER}/{tag_name}",
)
if not s3_folders_contents:
# TODO: Raise more specific exception
# TODO T122315363: Raise more specific exception
raise Exception(
f"Folder name {self.S3_LOGGING_FOLDER}/{tag_name} not found in bucket {s3_bucket_name}."
f"Please check if tag name {tag_name} and S3 bucket name {s3_bucket_name} are passed set correctly."
Expand All @@ -226,7 +226,7 @@ def download_logs(
)

# check for download folder in local
# TODO: Use pathlib instead of creating paths ourselves
# TODO T122315644: Use pathlib instead of creating paths ourselves
local_path = f"{local_download_dir}/{tag_name}"

self.utils.create_folder(folder_location=local_path)
Expand Down Expand Up @@ -271,9 +271,9 @@ def upload_logs_to_s3_from_cloudwatch(
if error.response.get("Error", {}).get("Code") == "NoSuchBucket":
error_message = f"Couldn't find bucket in the AWS account.\n{error}\n"
else:
# TODO: This error message doesn't seem right
# TODO T122315973: This error message doesn't seem right
error_message = "Couldn't find the S3 bucket in AWS account. Please use the right AWS S3 bucket name\n"
# TODO: Raise more specific exception
# TODO T122315363: Raise more specific exception
raise Exception(f"{error_message}")

# create logging folder
Expand Down Expand Up @@ -314,7 +314,7 @@ def upload_logs_to_s3_from_cloudwatch(
if not self._verify_log_stream(
log_group_name=log_group_name, log_stream_name=log_stream_name
):
# TODO: Raise more specific exception
# TODO T122315363: Raise more specific exception
raise Exception(
f"Couldn't find log stream {log_stream_name} in log group {log_group_name}"
)
Expand Down Expand Up @@ -343,7 +343,7 @@ def _parse_container_arn(self, container_arn: Optional[str]) -> List[str]:
service_name, container_name, container_id = None, None, None

if container_arn is None:
# TODO: Raise more specific exception
# TODO T122315363: Raise more specific exception
raise Exception(
"Container arn is missing. Please check the arn of the container"
)
Expand All @@ -359,10 +359,10 @@ def _parse_container_arn(self, container_arn: Optional[str]) -> List[str]:
task_id = container_arn_list[5]
container_name, container_id = self._get_container_name_id(task_id=task_id)
except IndexError as error:
# TODO: Raise more specific exception
# TODO T122315363: Raise more specific exception
raise Exception(f"Error in getting service name and task ID: {error}")

# TODO: Return dataclass object instead of list
# TODO T122316416: Return dataclass object instead of list
return [service_name, container_name, container_id]

def _parse_log_events(self, log_events: List[Dict[str, Any]]) -> List[str]:
Expand Down Expand Up @@ -402,12 +402,12 @@ def _get_container_name_id(self, task_id: str) -> List[str]:
container_name = task_id_list[1].replace("-cluster", "-container")
container_id = task_id_list[2]
except IndexError as error:
# TODO: Raise more specific exception
# TODO T122315363: Raise more specific exception
raise Exception(
f"Error in getting container name and container ID: {error}"
)

# TODO: Return dataclass object instead of list
# TODO T122316416: Return dataclass object instead of list
return [container_name, container_id]

def _verify_log_group(self, log_group_name: str) -> bool:
Expand Down Expand Up @@ -443,7 +443,7 @@ def _verify_log_group(self, log_group_name: str) -> bool:
f"Unexpected error occurred in fetching log group name {log_group_name}.\n"
f"{error}\n"
)
# TODO: Raise more specific exception
# TODO T122315363: Raise more specific exception
raise Exception(f"{error_message}")

return len(response.get("logGroups", [])) == 1
Expand Down Expand Up @@ -483,7 +483,7 @@ def _verify_log_stream(self, log_group_name: str, log_stream_name: str) -> bool:
f"Unexpected error occurred in finding log stream name {log_stream_name} in log grpup {log_group_name}\n"
f"{error}\n"
)
# TODO: Raise more specific exception
# TODO T122315363: Raise more specific exception
raise Exception(f"{error_message}")

return len(response.get("logStreams", [])) == 1
Expand Down

0 comments on commit 0706c21

Please sign in to comment.