#41 Introduce the packagers_without_bugzilla toddler
Merged 3 years ago by pingou. Opened 3 years ago by pingou.

@@ -0,0 +1,396 @@ 

+ from unittest.mock import call, Mock, patch

+ 

+ import pytest

+ 

+ from toddlers.plugins.packagers_without_bugzilla import main, PackagersWithoutBugzilla

+ 

+ 

+ class TestPackagersWithoutBugzillaToddler:

+ 

+     toddler_cls = PackagersWithoutBugzilla

+ 

+     def test_accepts_topic_invalid(self, toddler):

+         assert toddler.accepts_topic("foo.bar") is False

+ 

+     @pytest.mark.parametrize(

+         "topic",

+         [

+             "org.fedoraproject.*.toddlers.trigger.packagers_without_bugzilla",

+             "org.fedoraproject.prod.toddlers.trigger.packagers_without_bugzilla",

+             "org.fedoraproject.stg.toddlers.trigger.packagers_without_bugzilla",

+         ],

+     )

+     def test_accepts_topic_valid(self, toddler, topic):

+         assert toddler.accepts_topic(topic)

+ 

+     def test_process_no_email_override(self, toddler, capsys):

+         with pytest.raises(KeyError, match=r"'email_overrides_file'"):

+             toddler.process(config={}, message=None, username=False)

+ 

+         out, err = capsys.readouterr()

+         assert out == "Failed to load the file containing the email-overrides\n"

+         assert err == ""

+ 

+     def test_process_no_email_override_file(self, toddler, capsys):

+         with pytest.raises(

+             FileNotFoundError, match=r"No such file or directory: 'test'"

+         ):

+             toddler.process(

+                 config={"email_overrides_file": "test"},

+                 message=None,

+                 username=False,

+             )

+ 

+         out, err = capsys.readouterr()

+         assert out == "Failed to load the file containing the email-overrides\n"

+         assert err == ""

+ 

+     @patch("toddlers.utils.fedora_account.set_fas", new=Mock(return_value=True))

+     @patch("toddlers.utils.bugzilla_system.set_bz", new=Mock(return_value=True))

+     @patch("toddlers.utils.notify.send_email")

+     @patch("toddlers.utils.bugzilla_system.get_user")

+     @patch("toddlers.utils.bugzilla_system.get_group_member")

+     @patch("toddlers.utils.fedora_account.get_bz_email_group")

+     @patch("toddlers.utils.fedora_account.get_bz_email_user")

+     @patch("toml.load")

+     def test_process(

+         self,

+         toml_load,

+         get_bz_email_user,

+         get_bz_email_group,

+         bz_get_group_member,

+         bz_get_user,

+         send_email,

+         toddler,

+     ):

+         toml_load.return_value = {}

+         get_bz_email_user.side_effect = [

+             "besser82@fp.o",

+             "churchyard@fp.o",

+             "dwmw2@fp.o",

+         ]

+         get_bz_email_group.side_effect = ["python-sig@lists.fp.o"]

+         bz_get_group_member.return_value = ["dwmw2@fp.o", "besser82@fp.o"]

+         bz_get_user.side_effect = [Exception("ahah"), False, False, False]

+ 

+         req = Mock()

+         req.ok = True

+         req.json.return_value = {

+             "rpms": {

+                 "0xFFFF": ["dwmw2"],

+                 "2048-cli": ["besser82"],

+                 "CuraEngine": ["@python-sig", "churchyard"],

+             }

+         }

+         toddler.requests_session.get.return_value = req

+ 

+         toddler.process(

+             config={

+                 "email_overrides_file": "test",

+                 "bugzilla_group": "fedora_contrib",

+                 "dist_git_url": "https://src.fp.o",

+                 "admin_email": "admin@fp.o",

+                 "mail_server": "mail_server",

+             },

+             message=None,

+             username=False,

+         )

+ 

+         toml_load.assert_called_with("test")

+         get_bz_email_user.assert_called()

+         get_bz_email_user.assert_has_calls(

+             calls=[call("besser82", {}), call("churchyard", {}), call("dwmw2", {})]

+         )

+         get_bz_email_group.assert_called()

+         get_bz_email_group.assert_has_calls(calls=[call("python-sig", {})])

+         bz_get_group_member.assert_called_with("fedora_contrib")

+         bz_get_user.assert_called()

+         bz_get_user.assert_has_calls(

+             calls=[

+                 call(user_email="churchyard@fp.o"),

+                 call(user_email="python-sig@lists.fp.o"),

+             ]

+         )

+         send_email.assert_called()

+         send_email.assert_has_calls(

+             calls=[

+                 call(

+                     to_addresses=["churchyard@fp.o"],

+                     from_address="admin@fp.o",

+                     subject="Fedora Account System and Bugzilla Mismatch",

+                     content="Hello churchyard,\n\n"

+                     "We have identified you[1] as either a Fedora packager or someone who has "

+                     "asked to\n"

+                     "be included in the CC list of tickets created for one or more component on\n"

+                     "bugzilla. Fedora packagers are granted special permissions on the Fedora bugs"

+                     " in\n"

+                     "bugzilla.\n"

+                     "However, to enable these functionalities (granting you these permissions or\n"

+                     "including you to the CC list of your packages of interest), we need to have "

+                     "your\n"

+                     "bugzilla email address stored in the Fedora Account System[2].\n"

+                     "At the moment you have:\n\n"

+                     "churchyard@fp.o\n\n"

+                     "which bugzilla is telling us is not an account in bugzilla.  If you could\n"

+                     "please set up an account in bugzilla with this address or change your email\n"

+                     "address on your Fedora Account to match an existing bugzilla account this "

+                     "would\n"

+                     "let us go forward.\n\n"

+                     "Note: this message is being generated by an automated script.  You'll "

+                     "continue\n"

+                     "getting this message until the problem is resolved.  Sorry for the\n"

+                     "inconvenience.\n\nThank you,\nThe Fedora Account System\nadmin@fp.o\n\n\n"

+                     "[1] the source of this information is the following JSON file:\n"

+                     "    https://src.fedoraproject.org/extras/pagure_bz.json\n"

+                     "    We are happy to tell you exactly which packages are linked to your "

+                     "account\n"

+                     "    if you wish.\n[2] https://admin.fedoraproject.org/accounts\n",

+                     mail_server="mail_server",

+                 ),

+                 call(

+                     to_addresses=["python-sig@lists.fp.o"],

+                     from_address="admin@fp.o",

+                     subject="Fedora Account System and Bugzilla Mismatch",

+                     content="Hello @python-sig,\n\n"

+                     "We have identified you[1] as either a Fedora packager or someone who has "

+                     "asked to\n"

+                     "be included in the CC list of tickets created for one or more component on\n"

+                     "bugzilla. Fedora packagers are granted special permissions on the Fedora bugs "

+                     "in\n"

+                     "bugzilla.\n"

+                     "However, to enable these functionalities (granting you these permissions or\n"

+                     "including you to the CC list of your packages of interest), we need to have "

+                     "your\n"

+                     "bugzilla email address stored in the Fedora Account System[2].\n"

+                     "At the moment you have:\n\n"

+                     "python-sig@lists.fp.o\n\n"

+                     "which bugzilla is telling us is not an account in bugzilla.  If you could\n"

+                     "please set up an account in bugzilla with this address or change your email\n"

+                     "address on your Fedora Account to match an existing bugzilla account this "

+                     "would\n"

+                     "let us go forward.\n\n"

+                     "Note: this message is being generated by an automated script.  You'll "

+                     "continue\n"

+                     "getting this message until the problem is resolved.  Sorry for the\n"

+                     "inconvenience.\n\nThank you,\nThe Fedora Account System\nadmin@fp.o\n\n\n"

+                     "[1] the source of this information is the following JSON file:\n"

+                     "    https://src.fedoraproject.org/extras/pagure_bz.json\n"

+                     "    We are happy to tell you exactly which packages are linked to your "

+                     "account\n"

+                     "    if you wish.\n[2] https://admin.fedoraproject.org/accounts\n",

+                     mail_server="mail_server",

+                 ),

+                 call(

+                     to_addresses=["admin@fp.o"],

+                     from_address="admin@fp.o",

+                     subject="Toddlers found some packagers without bugzilla account",

+                     content="Dear Admin,\n\n"

+                     "The packagers_without_bugzilla toddler just ran and noticed some packagers "

+                     "without\n"

+                     "valid bugzilla account. The list of them is here:\n"

+                     "- churchyard (email: churchyard@fp.o) has no corresponding bugzilla account\n"

+                     "- @python-sig (email: python-sig@lists.fp.o) has no corresponding bugzilla "

+                     "account\n\n"

+                     "Each person on this list has been notified and, hopefully will fix the "

+                     "situation.\n"

+                     "Failing to do so, this notification may be used a record for a "

+                     "potential\n"

+                     "non-responsive packager procedure.\n\n"

+                     "Have a wonderful day and see you (maybe?) at the next run!\n\n",

+                     mail_server="mail_server",

+                 ),

+             ]

+         )

+ 

+     @patch("toddlers.utils.fedora_account.set_fas", new=Mock(return_value=True))

+     @patch("toddlers.utils.bugzilla_system.set_bz", new=Mock(return_value=True))

+     @patch("toddlers.utils.notify.send_email")

+     @patch("toddlers.utils.bugzilla_system.get_user")

+     @patch("toddlers.utils.bugzilla_system.get_group_member")

+     @patch("toddlers.utils.fedora_account.get_bz_email_group")

+     @patch("toddlers.utils.fedora_account.get_bz_email_user")

+     @patch("toml.load")

+     def test_process_username_no_bz_email(

+         self,

+         toml_load,

+         get_bz_email_user,

+         get_bz_email_group,

+         bz_get_group_member,

+         bz_get_user,

+         send_email,

+         toddler,

+     ):

+         toml_load.return_value = {}

+         get_bz_email_user.side_effect = [None]

+         get_bz_email_group.side_effect = ["python-sig@lists.fp.o"]

+         bz_get_group_member.return_value = ["dwmw2@fp.o", "besser82@fp.o"]

+         bz_get_user.return_value = False

+ 

+         req = Mock()

+         req.ok = True

+         req.json.return_value = {

+             "rpms": {

+                 "0xFFFF": ["dwmw2"],

+                 "2048-cli": ["besser82"],

+                 "CuraEngine": ["@python-sig", "churchyard"],

+             }

+         }

+         toddler.requests_session.get.return_value = req

+ 

+         toddler.process(

+             config={

+                 "email_overrides_file": "test",

+                 "bugzilla_group": "fedora_contrib",

+                 "dist_git_url": "https://src.fp.o",

+                 "admin_email": "admin@fp.o",

+                 "mail_server": "mail_server",

+             },

+             message=None,

+             username="nils",

+         )

+ 

+         toml_load.assert_called_with("test")

+         bz_get_group_member.assert_called_with("fedora_contrib")

+         get_bz_email_user.assert_called()

+         get_bz_email_user.assert_has_calls(calls=[call("nils", {})])

+         get_bz_email_group.assert_not_called()

+ 

+         bz_get_group_member.assert_called_with("fedora_contrib")

+         bz_get_user.assert_not_called()

+         send_email.assert_not_called()

+ 

+     @patch("toddlers.utils.fedora_account.set_fas", new=Mock(return_value=True))

+     @patch("toddlers.utils.bugzilla_system.set_bz", new=Mock(return_value=True))

+     @patch("toddlers.utils.notify.send_email")

+     @patch("toddlers.utils.bugzilla_system.get_user")

+     @patch("toddlers.utils.bugzilla_system.get_group_member")

+     @patch("toddlers.utils.fedora_account.get_bz_email_group")

+     @patch("toddlers.utils.fedora_account.get_bz_email_user")

+     @patch("toml.load")

+     def test_process_username_ignored(

+         self,

+         toml_load,

+         get_bz_email_user,

+         get_bz_email_group,

+         bz_get_group_member,

+         bz_get_user,

+         send_email,

+         toddler,

+     ):

+         toml_load.return_value = {}

+         get_bz_email_user.side_effect = [

+             "dwmw2@fp.o",

+         ]

+         get_bz_email_group.side_effect = ["python-sig@lists.fp.o"]

+         bz_get_group_member.return_value = ["churchyard@fp.o"]

+         bz_get_user.return_value = False

+ 

+         req = Mock()

+         req.ok = True

+         req.json.return_value = {"rpms": {"0xFFFF": ["dwmw2"]}}

+         toddler.requests_session.get.return_value = req

+ 

+         toddler.process(

+             config={

+                 "email_overrides_file": "test",

+                 "bugzilla_group": "fedora_contrib",

+                 "dist_git_url": "https://src.fp.o",

+                 "admin_email": "admin@fp.o",

+                 "mail_server": "mail_server",

+                 "ignorable_accounts": ["dwmw2"],

+             },

+             message=None,

+         )

+ 

+         toml_load.assert_called_with("test")

+         bz_get_group_member.assert_called_with("fedora_contrib")

+         get_bz_email_user.assert_called()

+         get_bz_email_user.assert_has_calls(calls=[call("dwmw2", {})])

+         get_bz_email_group.assert_not_called()

+         bz_get_user.assert_called()

+         bz_get_user.assert_has_calls(calls=[call(user_email="dwmw2@fp.o")])

+         send_email.assert_not_called()

+ 

+     @patch("toddlers.utils.fedora_account.set_fas", new=Mock(return_value=True))

+     @patch("toddlers.utils.bugzilla_system.set_bz", new=Mock(return_value=True))

+     @patch("toddlers.utils.notify.send_email")

+     @patch("toddlers.utils.bugzilla_system.get_user")

+     @patch("toddlers.utils.bugzilla_system.get_group_member")

+     @patch("toddlers.utils.fedora_account.get_bz_email_group")

+     @patch("toddlers.utils.fedora_account.get_bz_email_user")

+     @patch("toml.load")

+     def test_process_username_group_no_bz_email(

+         self,

+         toml_load,

+         get_bz_email_user,

+         get_bz_email_group,

+         bz_get_group_member,

+         bz_get_user,

+         send_email,

+         toddler,

+     ):

+         toml_load.return_value = {}

+         get_bz_email_group.side_effect = [None]

+         bz_get_group_member.return_value = ["dwmw2@fp.o", "besser82@fp.o"]

+         bz_get_user.return_value = False

+ 

+         req = Mock()

+         req.ok = True

+         req.json.return_value = {

+             "rpms": {

+                 "0xFFFF": ["dwmw2"],

+                 "2048-cli": ["besser82"],

+                 "CuraEngine": ["@python-sig", "churchyard"],

+             }

+         }

+         toddler.requests_session.get.return_value = req

+ 

+         toddler.process(

+             config={

+                 "email_overrides_file": "test",

+                 "bugzilla_group": "fedora_contrib",

+                 "dist_git_url": "https://src.fp.o",

+                 "admin_email": "admin@fp.o",

+                 "mail_server": "mail_server",

+             },

+             message=None,

+             username="@python-sig",

+         )

+ 

+         toml_load.assert_called_with("test")

+         bz_get_group_member.assert_called_with("fedora_contrib")

+         get_bz_email_user.assert_not_called()

+         get_bz_email_group.assert_called()

+         get_bz_email_group.assert_has_calls(calls=[call("python-sig", {})])

+         bz_get_group_member.assert_called_with("fedora_contrib")

+         bz_get_user.assert_not_called()

+         send_email.assert_not_called()

+ 

+     def test_main_no_args(self, capsys):

+         with pytest.raises(SystemExit):

+             main([])

+ 

+         out, err = capsys.readouterr()

+         assert out == ""

+         # Expecting something along these lines, but don't make the test too tight:

+         #

+         # usage: pytest [-h] [--dry-run] [-q | --debug] conf [username]

+         # pytest: error: the following arguments are required: conf

+         assert err.startswith("usage:")

+         assert "error: the following arguments are required:" in err

+ 

+     @patch("toml.load", new=Mock(return_value={}))

+     def test_main_debug(self, capsys):

+         with pytest.raises(KeyError, match=r"'email_overrides_file'"):

+             main(["test.cfg", "--debug"])

+         out, err = capsys.readouterr()

+         assert out == "Failed to load the file containing the email-overrides\n"

+         assert err == ""

+ 

+     @patch("toml.load", new=Mock(return_value={}))

+     def test_main(self, capsys):

+         with pytest.raises(KeyError, match=r"'email_overrides_file'"):

+             main(["test.cfg"])

+         out, err = capsys.readouterr()

+         assert out == "Failed to load the file containing the email-overrides\n"

+         assert err == ""

file modified
+5 -4
@@ -103,15 +103,16 @@ 

  # 7) the command should print out your token.

  token = "PUT_HERE_THE_TOKEN_OBTAINED_MANUALLY"

  

+ #  Configuration file storing all the email overrides in the form of:

Nice that this is moved to general section, I will use this in distgit_bugzilla_sync toddler

+ # "foo@bar.com" = "bar@foo.org"

+ # This is the same format as used by the distgit_bugzilla_sync cron/app

+ email_overrides_file = "/path/to/email_overrides.toml"

+ 

  [consumer_config.flag_ci_pr]

  

  [consumer_config.flag_commit_build]

  

  [consumer_config.packager_bugzilla_sync]

- #  Configuration file storing all the email overrides in the form of:

- # "foo@bar.com" = "bar@foo.org"

- # This is the same format as used by the distgit_bugzilla_sync cron/app

- email_overrides_file = "/path/to/email_overrides.toml"

  

  # List of accounts we do not want to report about

  ignorable_accounts = ["packagerbot", "zuul"]

@@ -0,0 +1,303 @@ 

+ """

+ This script is triggered by fedora-messaging messages published under the topic

+ ``toddlers.trigger.packager_without_bugzilla`` and checks if all packagers

+ currently maintaining packages have a corresponding bugzilla account.

+ 

+ Authors:    Pierre-Yves Chibon <pingou@pingoured.fr>

+ 

+ """

+ 

+ import argparse

+ import logging

+ import sys

+ import time

+ 

+ import toml

+ 

+ try:

+     import tqdm

+ except ImportError:

+     tqdm = None

+ 

+ from ..base import ToddlerBase

+ from ..utils import bugzilla_system

+ from ..utils import fedora_account

+ from ..utils import notify

+ from ..utils.requests import make_session

I'm still wondering about moving away from this to the usual full-path imports as this makes running the toddler manually, as a stand alone script not working :(
But I know @nphilipp likes this pattern :(

@pingou It's less that I like relative imports (they look odd to me, too) and more that I dislike importing with absolute paths because it caused me trouble in the past :wink:. It's ambiguous if you have e.g. a system-wide installed toddlers package from where it picks up your imports if you import or run the file from a checked out repo, depending on how you run it, or from where, it can pick up one or the other.

Currently, only two toddlers have code which historically allowed executing them directly. Rather than that, which would add very similar boilerplate code to every toddler, I'd add a shared API to toddlers which lets them set up command line options and be run directly. What do you think?

+ 

+ 

+ _log = logging.getLogger(__name__)

+ 

+ 

+ def get_bugzilla_user_with_retries(user_email, cnt):

+     try:

+         return bugzilla_system.get_user(user_email=user_email)

+     except Exception:

+         if cnt == 5:

+             raise  # pragma no cover

+         else:

+             time.sleep(2)

+             cnt = cnt + 1

+             return get_bugzilla_user_with_retries(user_email, cnt)

+ 

+ 

+ class PackagersWithoutBugzilla(ToddlerBase):

+     """Listens to messages sent by playtime (which lives in toddlers) to checks

+     that all packagers have a valid bugzilla account.

+     """

+ 

+     name = "packagers_without_bugzilla"

+ 

+     amqp_topics = [

+         "org.fedoraproject.*.toddlers.trigger.packagers_without_bugzilla",

+     ]

+ 

+     def __init__(self):

+         self.requests_session = make_session()

+         self.logs = []

+ 

+     def accepts_topic(self, topic):

+         """Returns a boolean whether this toddler is interested in messages

+         from this specific topic.

+         """

+         return topic.startswith("org.fedoraproject.") and topic.endswith(

+             "toddlers.trigger.packagers_without_bugzilla"

+         )

+ 

+     def get_user_and_groups_dist_git(self, dist_git_url):

+         """Returns a tuple of list containing in the first one all the users and

+         in the second all the groups found in dist-git in the JSON file meant to be

+         synced to bugzilla.

+         """

+         dist_git_url = dist_git_url.rstrip("/")

+         url = f"{dist_git_url}/extras/pagure_bz.json"

+         req = self.requests_session.get(url)

+         data = req.json()

+ 

+         users = set()

+         groups = set()

+         for namespace in data:

+             for package in data[namespace]:

+                 for name in data[namespace][package]:

+                     if name.startswith("@"):

+                         groups.add(name[1:])

+                     else:

+                         users.add(name)

+ 

+         return (users, groups)

+ 

+     def process(self, config, message, send_email=True, username=None):

+         """ Looks for packagers/groups without bugzilla email. """

+ 

+         try:

+             email_overrides = toml.load(config["email_overrides_file"])

+         except Exception:

+             print("Failed to load the file containing the email-overrides")

+             raise

+ 

+         _log.info("Setting up connection to FAS")

+         fedora_account.set_fas(config)

+ 

+         if not username:

+             # Retrieve all the packagers and groups in dist-git

+             _log.info("Retrieving the list of packagers and group in dist-git")

+             fas_packagers, fas_groups = self.get_user_and_groups_dist_git(

+                 config["dist_git_url"]

+             )

+         else:

+             if username.startswith("@"):

+                 fas_groups = [username[1:]]

+                 fas_packagers = []

+             else:

+                 fas_groups = []

+                 fas_packagers = [username]

+ 

+         n_packagers = len(fas_packagers)

+         n_groups = len(fas_groups)

+         _log.info("%s packagers found on dist-git", n_packagers)

+         _log.info("%s groups found on dist-git", n_groups)

+ 

+         fas_packagers_info = {}

+         fas_groups_info = {}

+         _log.info("Retrieving the bugzilla email for each packager")

+ 

+         # If the import fails, no progress bar

+         # At DEBUG or below, we're showing things at each iteration so the progress

+         # bar doesn't look good.

+         # At WARNING or above, we do not want to show anything.

+         if (

+             tqdm is not None and _log.getEffectiveLevel() == logging.INFO

+         ):  # pragma no cover

+             fas_packagers = tqdm.tqdm(fas_packagers)

+ 

+         for idx, username in enumerate(sorted(fas_packagers)):

+             _log.debug(

+                 "   Retrieving bz email of user %s: %s/%s", username, idx, n_packagers

+             )

+             bz_email = fedora_account.get_bz_email_user(username, email_overrides)

+             _log.debug("%s has email: %s", username, bz_email)

+             if bz_email:

+                 fas_packagers_info[bz_email] = username

+             else:

+                 _log.debug(

+                     "   -> Could not find a bugzilla email associated with: %s",

+                     username,

+                 )

+ 

+         for idx, groupname in enumerate(sorted(fas_groups)):

+             _log.debug(

+                 "   Retrieving bz email of group %s: %s/%s", groupname, idx, n_groups

+             )

+             bz_email = fedora_account.get_bz_email_group(groupname, email_overrides)

+             if bz_email:

+                 fas_groups_info[bz_email] = "@%s" % groupname

+             else:

+                 _log.debug(

+                     "   -> Could not find a bugzilla email associated with: @%s",

+                     groupname,

+                 )

+ 

+         _log.info("Setting up connection to bugzilla")

+         bugzilla_system.set_bz(config)

+ 

+         # Retrieve all the packagers in bugzilla

+         _log.info("Retrieving the list of packagers in bugzilla")

+         bz_packagers = bugzilla_system.get_group_member(config["bugzilla_group"])

+         n_bz_packagers = len(bz_packagers)

+         _log.info(

+             "%s members of %s found in bugzilla",

+             n_bz_packagers,

+             config["bugzilla_group"],

+         )

+ 

+         fas_set = set(fas_packagers_info) | set(fas_groups_info)

+         bz_set = set(bz_packagers)

+ 

+         overlap = len(fas_set.intersection(bz_set))

+         fas_only = fas_set - bz_set

+ 

+         _log.info("%s packagers found in both places", overlap)

+         _log.info("%s packagers found only in FAS (to be checked)", len(fas_only))

+ 

+         # Store a list of user with no bugzilla account

+         no_bz_account = []

+         for user_email in sorted(fas_only):

+ 

+             if not get_bugzilla_user_with_retries(user_email, 0):

+                 name = fas_packagers_info.get(user_email)

+                 if not name:

+                     name = fas_groups_info.get(user_email)

+                 if name in (config.get("ignorable_accounts") or []):

+                     continue

+ 

+                 info = f"{name} (email: {user_email}) has no corresponding bugzilla account"

+                 self.logs.append(info)

+                 _log.info(info)

+ 

+                 if send_email:

+                     _log.info(f"  Sending email to {user_email}")

+                     notify.notify_packager(

+                         config["mail_server"],

+                         config["admin_email"],

+                         username=name,

+                         email=user_email,

+                     )

+ 

+                 no_bz_account.append(user_email)

+ 

+         _log.info("%s emails had no corresponding bugzilla account", len(no_bz_account))

+ 

+         if self.logs:

+             logs_text = "\n- ".join(self.logs)

+             notify.notify_admins_on_packagers_without_bugzilla_accounts(

+                 to_addresses=[config["admin_email"]],

+                 from_address=config["admin_email"],

+                 mail_server=config["mail_server"],

+                 logs_text=logs_text,

+             )

+ 

+ 

+ # We have had the situation in the past where we've had to check a specific

+ # account, so the following code allows to run this script stand-alone if

+ # needed.

+ 

+ 

+ def setup_logging(log_level: int):

+     handlers = []

+ 

+     _log.setLevel(log_level)

+     # We want all messages logged at level INFO or lower to be printed to stdout

+     info_handler = logging.StreamHandler(stream=sys.stdout)

+     handlers.append(info_handler)

+ 

+     if log_level == logging.INFO:

+         # In normal operation, don't decorate messages

+         for handler in handlers:

+             handler.setFormatter(logging.Formatter("%(message)s"))

+ 

+     logging.basicConfig(level=log_level, handlers=handlers)

+ 

+ 

+ def get_arguments(args):

+     """ Load and parse the CLI arguments."""

+     parser = argparse.ArgumentParser(

+         description="Checks that packagers have a valid bugzilla account"

+     )

+     parser.add_argument(

+         "conf",

+         help="Configuration file",

+     )

+     parser.add_argument(

+         "--send-email",

+         action="store_true",

+         dest="send_email",

+         default=False,

+         help="Notify the packager(s) about their lack of account in bugzilla.",

+     )

+ 

+     parser.add_argument(

+         "username",

+         default=None,

+         nargs="?",

+         help="Process a specific user instead of all the packagers",

+     )

+ 

+     log_level_group = parser.add_mutually_exclusive_group()

+     log_level_group.add_argument(

+         "-q",

+         "--quiet",

+         action="store_const",

+         dest="log_level",

+         const=logging.WARNING,

+         default=logging.INFO,

+         help="Be less talkative",

+     )

+     log_level_group.add_argument(

+         "--debug",

+         action="store_const",

+         dest="log_level",

+         const=logging.DEBUG,

+         help="Enable debugging output",

+     )

+ 

+     return parser.parse_args(args)

+ 

+ 

+ def main(args):

+     """ Schedule the first test and run the scheduler. """

+     args = get_arguments(args)

+     setup_logging(log_level=args.log_level)

+ 

+     config = toml.load(args.conf)

+     PackagersWithoutBugzilla().process(

+         config=config.get("consumer_config", {}).get("packagers_without_bugzilla", {}),

+         message={},

+         username=args.username,

+     )

+ 

+ 

+ if __name__ == "__main__":  # pragma: no cover

+     try:

+         main(sys.argv[1:])

+     except KeyboardInterrupt:

+         pass

file modified
+25
@@ -85,6 +85,31 @@ 

      )

  

  

+ def notify_admins_on_packagers_without_bugzilla_accounts(

+     to_addresses, from_address, mail_server, logs_text

+ ):

+     message = f"""Dear Admin,

+ 

+ The packagers_without_bugzilla toddler just ran and noticed some packagers without

+ valid bugzilla account. The list of them is here:

+ - {logs_text}

+ 

+ Each person on this list has been notified and, hopefully will fix the situation.

+ Failing to do so, this notification may be used a record for a potential

+ non-responsive packager procedure.

+ 

+ Have a wonderful day and see you (maybe?) at the next run!

+ 

+ """

+     send_email(

+         to_addresses=to_addresses,

+         from_address=from_address,

+         subject="Toddlers found some packagers without bugzilla account",

+         content=message,

+         mail_server=mail_server,

+     )

+ 

+ 

  def send_email(to_addresses, from_address, subject, content, mail_server):

      """Actually sends the email to the list of addresses specified from the

      address given with the subject and content and via the given email server.

This toddler checks if all packagers have a bugzilla account and if they
do not, report it and notify them.

Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr

Build failed.

  • tox : FAILURE in 8m 45s

rebased onto 23639b6c684d102af17f845388157c6fe4503d83

3 years ago

Build failed.

  • tox : FAILURE in 10m 09s

rebased onto 57da9ea19ccb0ef61ac7ba42fbc96a071e09ff29

3 years ago

Build succeeded.

  • tox : SUCCESS in 6m 23s

Nice that this is moved to general section, I will use this in distgit_bugzilla_sync toddler

I saw this on plenty of places in distgit_bugzilla_sync. I would rather have some function or wrapper around bugzilla calls we are using. I don't really like the definitions of functions inside methods. I think it makes the code less readable

Why not adding the new function to notify module like it is done for packager_distgit_sync toddler.

Few notes otherwise looks good.

rebased onto c30c3a45988b7e29c696987af56d0eec885e70a4

3 years ago

We could, I find that it makes the toddler a little less self-contained, so I've started putting these in the toddler itself (there is another example where the email is in the toddler).

I guess we should pick one and stick with it.

I guess we could move it. Will do that later

I would be for having the messages outside of toddlers. It makes them smaller and easier to read.

Build succeeded.

  • tox : SUCCESS in 6m 55s

rebased onto eefbeddcd88eaedb297bd9486effeba1bf90bbdd

3 years ago

I would be for having the messages outside of toddlers. It makes them smaller and easier to read.

Adjusted :)

rebased onto bf35650

3 years ago

I would be for having the messages outside of toddlers. It makes them smaller and easier to read.

Adjusted :)

Much better and more readable.

I'm still wondering about moving away from this to the usual full-path imports as this makes running the toddler manually, as a stand alone script not working :(
But I know @nphilipp likes this pattern :(

I think we can merge this.

Build succeeded.

  • tox : SUCCESS in 5m 53s

Let's get it in then :)

I have a PR coming to move more of the email sending code to the notify module

Pull-Request has been merged by pingou

3 years ago

@pingou It's less that I like relative imports (they look odd to me, too) and more that I dislike importing with absolute paths because it caused me trouble in the past :wink:. It's ambiguous if you have e.g. a system-wide installed toddlers package from where it picks up your imports if you import or run the file from a checked out repo, depending on how you run it, or from where, it can pick up one or the other.

Currently, only two toddlers have code which historically allowed executing them directly. Rather than that, which would add very similar boilerplate code to every toddler, I'd add a shared API to toddlers which lets them set up command line options and be run directly. What do you think?

@nphilipp This sounds awesome, I'm working on another toddler that could benefit from being run directly in command line.