email: update cc list to enable multiple recipients

[peluse]

Actually, I was getting an error with just one email. Further below From my change on this LOC:

for to in to_list + cc_list:

I was getting an error about concatenating a list and a string. The to_list Was clearly a list based on how it was processed just above the for loop.

The cc_list printed its type as a list but it looked like a string logging it.

Either way, single or multiple entry cc in the config file, this patch works.

[facebook-github-bot]

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[chantra]

Thanks @peluse .

Are you passing a list to the email's cc entry? Or a comma-separated list of emails.

The former is expected and looking at your patch, I feel we should rather raise in

kernel-patches-daemon/kernel_patches_daemon/config.py

Line 125 in 827c607

return cls(

if either isinstance(smtp_to, list) or isinstance(smtp_cc, list) is False.

so the message is clear has to why this is failing, and we don't have to deal with a mix of list or comma-separated list in the rest of the code.

[danobi]

Yeah for reference the email config should look something like this:

  "email": {
    "host": <REDACTED>
    "port": <REDACTED>
    "user": "bot-bpf-ci",
    "from": "[email protected]",
    "cc": [
      "[email protected]",
      "[email protected]",
      "[email protected]",
      "[email protected]"
    ],
    "pass": <REDACTED>
    "http_proxy": <REDACTED>
    "submitter_allowlist": [
      ".*"
    ],
    "ignore_allowlist": false
  },

I'd check that you're passing the right json thingy into "cc".

We should probably follow up with better documentation if we don't have that already.

[chantra]

We should probably follow up with better documentation if we don't have that already.

Totally. I was mentioning to @theihor to update the documentation has he goes through setting his own testing repository.

Tests are currently failing for some other reasons (a revert should make it to main soon), but this got caught in our internal CI with:

test_email_submitter_not_in_allowlist (kernel.kernel_patches_daemon.oss.tests.test_branch_worker.TestEmailNotification) ... ERROR

======================================================================
ERROR: test_email_submitter_not_in_allowlist (kernel.kernel_patches_daemon.oss.tests.test_branch_worker.TestEmailNotification)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/path/to/repo/kernel/kernel_patches_daemon/oss/tests/__test_branch_worker__/test_branch_worker#link-tree/kernel/kernel_patches_daemon/oss/tests/test_branch_worker.py", line 1332, in test_email_submitter_not_in_allowlist
    cmd, email = build_email(
  File "/path/to/repo/kernel/kernel_patches_daemon/oss/tests/__test_branch_worker__/test_branch_worker#link-tree/kernel_patches_daemon/branch_worker.py", line 291, in build_email
    cc_list = copy.copy(config.smtp_cc).split(",")
AttributeError: 'list' object has no attribute 'split'

----------------------------------------------------------------------
Ran 1 test in 0.002s

FAILED (errors=1)

AttributeError: 'list' object has no attribute 'split'
  File "/path/to/python/lib/python3.10/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/path/to/python/lib/python3.10/unittest/case.py", line 591, in run
    self._callTestMethod(testMethod)
  File "/path/to/python/lib/python3.10/unittest/case.py", line 549, in _callTestMethod
    method()
  File "/path/to/repo/kernel/kernel_patches_daemon/oss/tests/__test_branch_worker__/test_branch_worker#link-tree/kernel/kernel_patches_daemon/oss/tests/test_branch_worker.py", line 1332, in test_email_submitter_not_in_allowlist
    cmd, email = build_email(
  File "/path/to/repo/kernel/kernel_patches_daemon/oss/tests/__test_branch_worker__/test_branch_worker#link-tree/kernel_patches_daemon/branch_worker.py", line 291, in build_email
    cc_list = copy.copy(config.smtp_cc).split(",")

[chantra]

The type checker (which I don't think we run here either, and probably should follow up) caught it too:

Test returned with non-zero exit code
stdout:

stderr:
Found type errors!
kernel/kernel_patches_daemon/oss/kernel_patches_daemon/branch_worker.py:291:14 Undefined attribute [16]: `typing.List` has no attribute `split`.

[peluse]

thanks guys! sorry was off for the last week. I was not using the format mentioned above and didn't see any examples so I was just putting one email address in there in quotes and it didn't work. Looking at what you suggest surely looks like it will work. I will test it out very soon and close this PR when it works ;)