Merge pull request #58 from stfc/yml_config

Change read methods to yaml

cloud-chatops/.gitignore

@@ -1,3 +1,5 @@
 .idea/
 __pycache__/
 tests/__pycache__/
+.coverage
+coverage.xml

cloud-chatops/requirements.txt

@@ -7,4 +7,5 @@ python-dateutil
 pytest
 pytest-cov
 pylint
-black
+black
+pyyaml

cloud-chatops/src/errors.py

@@ -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):

cloud-chatops/src/features/base_feature.py

@@ -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
@@ -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:
         """
@@ -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),

cloud-chatops/src/get_github_prs.py

@@ -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]:
@@ -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
 

cloud-chatops/src/main.py

@@ -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"
@@ -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(

cloud-chatops/src/read_data.py

@@ -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,
@@ -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/"
+
 except IndexError:
     pass
+
 except KeyError:
     print(
         "Are you trying to run locally? Couldn't find HOME in your environment variables."
@@ -31,9 +38,9 @@ def validate_required_files() -> None:
     """
     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.")
 
     tokens = ["SLACK_BOT_TOKEN", "SLACK_APP_TOKEN", "GITHUB_TOKEN"]
     for token in tokens:
@@ -42,12 +49,16 @@ def validate_required_files() -> None:
             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:
@@ -62,37 +73,16 @@ def get_token(secret: str) -> str:
     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)

cloud-chatops/tests/test_base_feature.py

@@ -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()

cloud-chatops/tests/test_get_github_prs.py

@@ -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 == []
 
 

cloud-chatops/tests/test_pr_message_builder.py

@@ -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()
 
@@ -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",
@@ -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",
@@ -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"

cloud-chatops/tests/test_pr_reminder.py

@@ -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()