sebbASF commented on code in PR #608:
URL:
https://github.com/apache/tooling-trusted-releases/pull/608#discussion_r2738633933
##########
atr/mail.py:
##########
@@ -15,22 +15,21 @@
# specific language governing permissions and limitations
# under the License.
+# Licensed to the Apache Software Foundation (ASF) under one... (License block)
+
Review Comment:
Why has this been added?
##########
atr/mail.py:
##########
@@ -143,7 +137,7 @@ def _split_address(addr: str) -> tuple[str, str]:
def _validate_recipient(to_addr: str) -> None:
- # Ensure recipient is @apache.org or @tooling.apache.org
+ """Ensure recipient is @apache.org or @tooling.apache.org."""
Review Comment:
Whilst this change is OK, it is not strictly part of the fix.
In future, please ensure PRs make minimal changes, and only address the
issue at hand.
Otherwise the PR is harder to review, and may be rejected for a change that
is nothing to do with the fix.
##########
atr/mail.py:
##########
@@ -56,34 +55,38 @@ async def send(message: Message) -> tuple[str, list[str]]:
to_addr = message.email_recipient
_validate_recipient(to_addr)
- # UUID4 is entirely random, with no timestamp nor namespace
- # It does have 6 version and variant bits, so only 122 bits are random
Review Comment:
Why has this been dropped?
##########
atr/mail.py:
##########
@@ -96,14 +99,12 @@ async def send(message: Message) -> tuple[str, list[str]]:
return mid, errors
-async def _send_many(from_addr: str, to_addrs: list[str], msg_text: str) ->
list[str]:
+async def _send_many(from_addr: str, to_addrs: list[str], msg_bytes: bytes) ->
list[str]:
Review Comment:
Why has this been changed from string to bytes?
##########
atr/mail.py:
##########
@@ -15,22 +15,21 @@
# specific language governing permissions and limitations
# under the License.
+# Licensed to the Apache Software Foundation (ASF) under one... (License block)
+
import dataclasses
import email.utils as utils
import ssl
import time
import uuid
+from email.headerregistry import Address
+from email.message import EmailMessage
from typing import Final
import aiosmtplib
-# import dkim
import atr.log as log
-# TODO: We should choose a pattern for globals
-# We could e.g. use uppercase instead of global_
-# It's not always worth identifying globals as globals
-# But in many cases we should do so
Review Comment:
Why has this been dropped?
##########
atr/mail.py:
##########
@@ -115,13 +116,6 @@ async def _send_via_relay(from_addr: str, to_addr: str,
msg_bytes: bytes) -> Non
"""Send an email to a single recipient via the ASF mail relay."""
_validate_recipient(to_addr)
- # Connect to the ASF mail relay
- # NOTE: Our code is very different from the asfpy code:
- # - Uses types
- # - Uses asyncio
- # Due to the divergence, we should probably not contribute upstream
- # In effect, these are two different "packages" of functionality
- # We can't even sign it first and pass it to asfpy, due to its different
design
Review Comment:
Why has this comment been dropped?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
