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

Change read methods to yaml #58

Merged
merged 14 commits into from
Oct 23, 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
2 changes: 2 additions & 0 deletions cloud-chatops/.gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
.idea/
__pycache__/
tests/__pycache__/
.coverage
coverage.xml
3 changes: 2 additions & 1 deletion cloud-chatops/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ python-dateutil
pytest
pytest-cov
pylint
black
black
pyyaml
2 changes: 1 addition & 1 deletion cloud-chatops/src/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class UnknownHTTPError(RuntimeError):


class RepositoriesNotGiven(RuntimeError):
"""Error: repos.csv does not contain any repositories."""
"""Error: repos supplied is empty does not contain any repositories."""


class TokensNotGiven(RuntimeError):
Expand Down
8 changes: 4 additions & 4 deletions cloud-chatops/src/features/base_feature.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from slack_sdk import WebClient
from slack_sdk.errors import SlackApiError
from dateutil import parser as datetime_parser
from read_data import get_token, get_repos, get_user_map
from read_data import get_token, get_config
from get_github_prs import GetGitHubPRs
from pr_dataclass import PrData
from errors import FailedToPostMessage, UserNotFound
Expand All @@ -33,8 +33,8 @@ class BaseFeature(ABC):
def __init__(self):
self.channel = DEFAULT_CHANNEL
self.client = WebClient(token=get_token("SLACK_BOT_TOKEN"))
self.prs = GetGitHubPRs(get_repos(), DEFAULT_REPO_OWNER).run()
self.slack_ids = get_user_map()
self.prs = GetGitHubPRs(get_config("repos")).run()
self.slack_ids = get_config("user-map")

def _post_reminder_message(self) -> str:
"""
Expand Down Expand Up @@ -175,7 +175,7 @@ def check_pr(self, info: PrData) -> PrData:
:param info: The information to validate.
:return: The validated information.
"""
slack_ids = get_user_map()
slack_ids = get_config("user-map")
new_info = replace(
info,
user=slack_ids.get(info.user, info.user),
Expand Down
17 changes: 10 additions & 7 deletions cloud-chatops/src/get_github_prs.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@ class GetGitHubPRs:
This class handles getting the open PRs from the GitHub Rest API.
"""

def __init__(self, repos: List[str], owner: str):
def __init__(self, repos: Dict[str, List]):
"""
This method initialises the class with the following attributes.
:param repos: A list of repositories to get pull requests for.
:param repos: The owner of the above repositories.
"""
self.repos = repos
self.owner = owner
self._http_handler = HTTPHandler()

def run(self) -> List[PrData]:
Expand All @@ -37,17 +36,21 @@ def run(self) -> List[PrData]:
It runs the HTTP request methods and returns the responses.
:return: The responses from the HTTP requests.
"""
responses = self._request_all_repos_http()
responses = []
for owner in self.repos:
responses += self._request_all_repos_http(owner, self.repos.get(owner))
return self._parse_pr_to_dataclass(responses)

def _request_all_repos_http(self) -> List[Dict]:
def _request_all_repos_http(self, owner: str, repos: List[str]) -> List[Dict]:
"""
This method starts a request for each repository and returns a list of those PRs.
:return: A dictionary of repos and their PRs.
:param owner: The organisation for repos
:param repos: List of repository names
:return: A list of PRs stored as dictionaries
"""
responses = []
for repo in self.repos:
url = f"https://api.github.com/repos/{self.owner}/{repo}/pulls"
for repo in repos:
url = f"https://api.github.com/repos/{owner}/{repo}/pulls"
responses += self._http_handler.make_request(url)
return responses

Expand Down
4 changes: 2 additions & 2 deletions cloud-chatops/src/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import schedule
from features.pr_reminder import PostPRsToSlack
from features.post_to_dms import PostToDMs
from read_data import get_token, validate_required_files, get_user_map
from read_data import get_token, validate_required_files, get_config


PULL_REQUESTS_CHANNEL = "C03RT2F6WHZ"
Expand Down Expand Up @@ -60,7 +60,7 @@ def run_global_reminder(channel) -> None:

def run_personal_reminder() -> None:
"""This is a placeholder function for the schedule to accept."""
users = list(get_user_map().values())
users = list(get_config("user-map").values())
PostToDMs().run(users=users, post_all=False)

schedule.every().monday.at("09:00").do(
Expand Down
66 changes: 28 additions & 38 deletions cloud-chatops/src/read_data.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
"""This module handles reading data from files such as secrets and user maps."""

from typing import List, Dict
from typing import Dict, Union
import sys
import os
import json
import yaml
from errors import (
RepositoriesNotGiven,
UserMapNotGiven,
Expand All @@ -13,13 +14,19 @@
# Production secret path
PATH = "/usr/src/app/cloud_chatops_secrets/"
try:
if sys.argv[1] == "local":
if sys.argv[1] == "dev" or sys.argv[1] == "development":
# Using dev secrets here for local testing as it runs the application
# in a separate Slack Workspace than the production application.
# This means the slash commands won't be picked up by the production application.
PATH = f"{os.environ['HOME']}/dev_cloud_chatops_secrets/"

elif sys.argv[1] == "prod" or sys.argv[1] == "production":
# Using prod secrets here in case the application is run directly on host without Docker.
PATH = f"{os.environ['HOME']}/cloud_chatops_secrets/"

Check warning on line 25 in cloud-chatops/src/read_data.py

View check run for this annotation

Codecov / codecov/patch

cloud-chatops/src/read_data.py#L25

Added line #L25 was not covered by tests

except IndexError:
pass

except KeyError:
print(
"Are you trying to run locally? Couldn't find HOME in your environment variables."
Expand All @@ -31,9 +38,9 @@
"""
This function checks that all required files have data in them before the application runs.
"""
repos = get_repos()
repos = get_config("repos")
if not repos:
raise RepositoriesNotGiven("repos.csv does not contain any repositories.")
raise RepositoriesNotGiven("config.yml does not contain any repositories.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't have any docs for how the config.yml should look in this PR? It requires someone to effectively dig the source code

Copy link
Member Author

Choose a reason for hiding this comment

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

Those changes are in #57 once it is merged and this one rebased then it will make sense.


tokens = ["SLACK_BOT_TOKEN", "SLACK_APP_TOKEN", "GITHUB_TOKEN"]
for token in tokens:
Expand All @@ -42,12 +49,16 @@
raise TokensNotGiven(
f"Token {token} does not have a value in secrets.json."
)
user_map = get_user_map()
user_map = get_config("user-map")
if not user_map:
raise UserMapNotGiven("user_map.json is empty.")
raise UserMapNotGiven("config.yml does not contain a user map is empty.")
for item, value in user_map.items():
if not value:
raise UserMapNotGiven(f"User {item} does not have a Slack ID assigned.")
if not item:
raise UserMapNotGiven(
f"Slack member {value} does not have a GitHub username assigned."
)


def get_token(secret: str) -> str:
Expand All @@ -62,37 +73,16 @@
return secrets[secret]


def get_repos() -> List[str]:
def get_config(section: str = "all") -> Union[Dict, str]:
"""
This function reads the repo csv file and returns a list of repositories
:return: List of repositories as strings
"""
with open(PATH + "repos.csv", "r", encoding="utf-8") as file:
data = file.read()
repos = data.split(",")
if not repos[-1]:
repos = repos[:-1]
return repos


def get_user_map() -> Dict:
This function will return the specified section from the config file.
:param section: The section of the config to retrieve.
:return: The data retrieved from the config file.
"""
This function gets the GitHub to Slack username mapping from the map file.
:return: Dictionary of username mapping
"""
with open(PATH + "user_map.json", "r", encoding="utf-8") as file:
data = file.read()
user_map = json.loads(data)
return user_map


def get_maintainer() -> str:
"""
This function will get the maintainer user's Slack ID from the text file.
:return: Slack Member ID
"""
with open(PATH + "maintainer.txt", "r", encoding="utf-8") as file:
data = file.read()
if not data:
return "U05RBU0RF4J" # Default Maintainer: Kalibh Halford
return data
with open(PATH + "config.yml", "r", encoding="utf-8") as config:
config_data = yaml.safe_load(config)
match section:
case "all":
return config_data
case _:
return config_data.get(section)
8 changes: 2 additions & 6 deletions cloud-chatops/tests/test_base_feature.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,14 @@
@patch("features.base_feature.WebClient")
@patch("features.base_feature.GetGitHubPRs")
@patch("features.base_feature.get_token")
@patch("features.base_feature.get_repos")
@patch("features.base_feature.get_user_map")
@patch("features.base_feature.get_config")
def instance_fixture(
mock_get_user_map,
mock_get_repos,
_,
mock_get_token,
mock_get_github_prs,
mock_web_client,
):
"""This fixture provides a class instance for the tests"""
mock_get_user_map.return_value = {"mock_github": "mock_slack"}
mock_get_repos.return_value = ["mock_repo1", "mock_repo2"]
mock_get_token.return_value = "mock_slack_token"
mock_get_github_prs.run.return_value = []
mock_web_client.return_value = NonCallableMock()
Expand Down
31 changes: 15 additions & 16 deletions cloud-chatops/tests/test_get_github_prs.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,45 +11,44 @@
@pytest.fixture(name="instance", scope="function")
def instance_fixture():
"""Creates a class fixture to use in the tests"""
mock_repos = ["repo1", "repo2"]
mock_owner = "mock_user"
return GetGitHubPRs(mock_repos, mock_owner)
mock_repos = {"owner1": ["repo1"]}
return GetGitHubPRs(mock_repos)


@patch("get_github_prs.GetGitHubPRs._request_all_repos_http")
@patch("get_github_prs.GetGitHubPRs._parse_pr_to_dataclass")
def test_run(mock_parse_pr_to_dataclass, mock_request_all_repos_http, instance):
"""Tests the run method returns the correct object"""
res = instance.run()
mock_request_all_repos_http.assert_called_once_with()
mock_parse_pr_to_dataclass.assert_called_once_with(
mock_request_all_repos_http.return_value
)
for owner in instance.repos:
mock_request_all_repos_http.assert_any_call(owner, instance.repos.get(owner))
mock_parse_pr_to_dataclass.assert_called_once()
assert res == mock_parse_pr_to_dataclass.return_value


@patch("get_github_prs.HTTPHandler.make_request")
def test_request_all_repos_http(mock_make_request, instance):
"""Test a request is made for each repo in the list"""
mock_owner = "owner1"
mock_repos = instance.repos.get(mock_owner)
mock_make_request.side_effect = [
[f"https://api.github.com/repos/{instance.owner}/{repo}/pulls"]
for repo in instance.repos
[f"https://api.github.com/repos/{mock_owner}/{repo}/pulls"]
for repo in mock_repos
]
res = instance._request_all_repos_http()
for repo in instance.repos:
res = instance._request_all_repos_http(mock_owner, mock_repos)
for repo in mock_repos:
mock_make_request.assert_any_call(
f"https://api.github.com/repos/{instance.owner}/{repo}/pulls"
f"https://api.github.com/repos/{mock_owner}/{repo}/pulls"
)
assert res == [
f"https://api.github.com/repos/{instance.owner}/{repo}/pulls"
for repo in instance.repos
f"https://api.github.com/repos/{mock_owner}/{repo}/pulls" for repo in mock_repos
]


def test_request_all_repos_http_none(instance):
"""Test that nothing is returned when no repos are given"""
instance.repos = []
res = instance._request_all_repos_http()
instance.repos = {}
res = instance._request_all_repos_http("", [])
assert res == []


Expand Down
19 changes: 10 additions & 9 deletions cloud-chatops/tests/test_pr_message_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@
@patch("features.base_feature.WebClient")
@patch("features.base_feature.GetGitHubPRs")
@patch("features.base_feature.get_token")
@patch("features.base_feature.get_repos")
@patch("features.base_feature.get_user_map")
def instance_fixture(_, _2, _3, _4, _5):
@patch("features.base_feature.get_config")
def instance_fixture(_, _2, _3, _4):
"""Provides a class instance for the tests"""
return PRMessageBuilder()

Expand Down Expand Up @@ -128,8 +127,8 @@ def test_check_pr_age_old(


@patch("features.base_feature.PRMessageBuilder._check_pr_age")
@patch("features.base_feature.get_user_map")
def test_check_pr_info_found_name(mock_user_map, mock_check_pr_age, instance):
@patch("features.base_feature.get_config")
def test_check_pr_info_found_name(mock_get_config, mock_check_pr_age, instance):
"""Test the dataclass is updated and name is found"""
mock_data = PrData(
pr_title="mock_title",
Expand All @@ -139,16 +138,17 @@ def test_check_pr_info_found_name(mock_user_map, mock_check_pr_age, instance):
draft=False,
old=False,
)
mock_user_map.return_value = {"mock_github": "mock_slack"}
mock_get_config.return_value = {"mock_github": "mock_slack"}
mock_check_pr_age.return_value = True
res = instance.check_pr(mock_data)
mock_get_config.assert_called_once_with("user-map")
assert res.user == "mock_slack"
assert res.old


@patch("features.base_feature.PRMessageBuilder._check_pr_age")
@patch("features.base_feature.get_user_map")
def test_check_pr_info_unfound_name(mock_user_map, _, instance):
@patch("features.base_feature.get_config")
def test_check_pr_info_unfound_name(mock_get_config, _, instance):
"""Test the dataclass is updated and name is not found"""
mock_data = PrData(
pr_title="mock_title",
Expand All @@ -158,6 +158,7 @@ def test_check_pr_info_unfound_name(mock_user_map, _, instance):
draft=False,
old=False,
)
mock_user_map.return_value = {"mock_github": "mock_slack"}
mock_get_config.return_value = {"mock_github": "mock_slack"}
res = instance.check_pr(mock_data)
mock_get_config.assert_called_once_with("user-map")
assert res.user == "mock_user"
8 changes: 2 additions & 6 deletions cloud-chatops/tests/test_pr_reminder.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,14 @@
@patch("features.base_feature.WebClient")
@patch("features.base_feature.GetGitHubPRs")
@patch("features.base_feature.get_token")
@patch("features.base_feature.get_repos")
@patch("features.base_feature.get_user_map")
@patch("features.base_feature.get_config")
def instance_fixture(
mock_get_user_map,
mock_get_repos,
_,
mock_get_token,
mock_get_github_prs,
mock_web_client,
):
"""This fixture provides a class instance for the tests"""
mock_get_user_map.return_value = {"mock_github": "mock_slack"}
mock_get_repos.return_value = ["mock_repo1", "mock_repo2"]
mock_get_token.return_value = "mock_slack_token"
mock_get_github_prs.run.return_value = []
mock_web_client.return_value = NonCallableMock()
Expand Down
Loading