Cleanups without additional features (#424)

* main: make github_repo and github_token_env main options

* graphql: minimize_comment: have default reason

* github_graphql: sent gzip header

* github_util: update doc string

* github_util: Use token we've determined before

* github_util: Raise more informative exception when editing a comment

* Fix tests

snapshot_manager/main.py

@@ -26,6 +26,24 @@ def main():
         **parser_args,
     )
 
+    mainparser.add_argument(
+        "--github-token-env",
+        metavar="ENV_NAME",
+        type=str,
+        dest="github_token_env",
+        default=cfg.github_token_env,
+        help="Default name of the environment variable which holds the github token",
+    )
+
+    mainparser.add_argument(
+        "--github-repo",
+        metavar="OWNER/REPO",
+        type=str,
+        dest="github_repo",
+        default=cfg.github_repo,
+        help="Repo where to open or update issues.",
+    )
+
     # For config file support see:
     # https://newini.wordpress.com/2021/06/11/how-to-import-config-file-to-argparse-using-configparser/
     # subparser_check.add_argument(
@@ -81,15 +99,6 @@ def main():
         help="Maintainer handle to use for assigning issues.",
     )
 
-    subparser_check.add_argument(
-        "--github-repo",
-        metavar="OWNER/REPO",
-        type=str,
-        dest="github_repo",
-        default=cfg.github_repo,
-        help="Repo where to open or update issues.",
-    )
-
     subparser_check.add_argument(
         "--copr-ownername",
         metavar="COPR-OWNWERNAME",
@@ -108,15 +117,6 @@ def main():
         help="Copr project name to check. 'YYYYMMDD' will be replaced, so make sure you have it in there.",
     )
 
-    subparser_check.add_argument(
-        "--github-token-env",
-        metavar="ENV_NAME",
-        type=str,
-        dest="github_token_env",
-        default=cfg.github_token_env,
-        help="Default name of the environment variable which holds the github token",
-    )
-
     subparser_check.add_argument(
         "--copr-monitor-tpl",
         metavar="COPR-MONITOR-TPL",

snapshot_manager/snapshot_manager/github_graphql.py

@@ -49,6 +49,8 @@ def __init__(
         self.__session.headers.update(
             {
                 "Authorization": f"Bearer {self.__token}",
+                # See https://graphql.org/learn/best-practices/#json-with-gzip
+                "Accept-Encoding": "gzip",
                 # See #
                 # https://github.blog/2021-11-16-graphql-global-id-migration-update/
                 "X-Github-Next-Global-ID": "1",

snapshot_manager/snapshot_manager/github_util.py

@@ -34,9 +34,7 @@ def __init__(self, config: config.Config, github_token: str = None, **kwargs):
         if github_token is None:
             github_token = os.getenv(self.config.github_token_env)
         self.github = github.Github(login_or_token=github_token)
-        self.gql = github_graphql.GithubGraphQL(
-            token=os.getenv(self.config.github_token_env), raise_on_error=True
-        )
+        self.gql = github_graphql.GithubGraphQL(token=github_token, raise_on_error=True)
         self.__label_cache = None
         self.__repo_cache = None
 
@@ -314,7 +312,12 @@ def create_or_update_comment(
         comment = self.get_comment(issue=issue, marker=marker)
         if comment is None:
             return issue.create_comment(body=comment_body)
-        comment.edit(body=comment_body)
+        try:
+            comment.edit(body=comment_body)
+        except github.GithubException.GithubException as ex:
+            raise ValueError(
+                f"Failed to update github comment with marker {marker} and comment body: {comment_body}"
+            ) from ex
         return comment
 
     def remove_labels_safe(
@@ -346,15 +349,17 @@ def minimize_comment_as_outdated(
         self,
         object: str | github.IssueComment.IssueComment,
     ) -> bool:
-        """Minimizes a comment with the given `node_id` and the reason `OUTDATED`.
+        """Minimizes a comment identified by the `object` argument with the reason `OUTDATED`.
 
         Args:
-            node_id (str): A comment's `node_id`.
+            object (str | github.IssueComment.IssueComment): The comment to minimize
+
+        Raises:
+            ValueError: If the `object` has a wrong type.
 
         Returns:
-            bool: True if the comment was minimized
+            bool: True if the comment was properly minimized.
         """
-
         node_id = ""
         if isinstance(object, github.IssueComment.IssueComment):
             node_id = object.raw_data["node_id"]

snapshot_manager/snapshot_manager/graphql/minimize_comment.gql

@@ -1,4 +1,4 @@
-mutation minimizeComment($id: ID!, $classifier: ReportedContentClassifiers!) {
+mutation minimizeComment($id: ID!, $classifier: ReportedContentClassifiers = OUTDATED) {
   minimizeComment(input: {subjectId: $id, classifier: $classifier}) {
     clientMutationId
     minimizedComment {

snapshot_manager/tests/github_util_test.py

@@ -50,7 +50,7 @@ def test_flip_test_label(self):
         self.assertIsNotNone(issue)
 
         # Remove all labels
-        logging.info("Removing all labels from issue")
+        logging.info(f"Removing all labels from issue: {issue.html_url}")
         for label in issue.get_labels():
             issue.remove_from_labels(label)
         self.assertEqual(issue.get_labels().totalCount, 0)
@@ -69,11 +69,14 @@ def test_flip_test_label(self):
 
         all_test_states = [in_testing, failed_on, tested_on]
         for test_state in all_test_states:
-            logging.info(f"Flipping test label for chroot {chroot}")
+            logging.info(f"Flipping test label for chroot {chroot} to: {test_state}")
             gh.flip_test_label(issue, chroot, test_state)
             labels = issue.get_labels()
+            self.assertIsNotNone(labels)
             self.assertEqual(labels.totalCount, 1)
-            self.assertEqual(labels.get_page(0)[0].name, test_state)
+            page = labels.get_page(0)
+            self.assertIsNotNone(page)
+            self.assertEqual(page[0].name, test_state)
         pass