Skip to content

Commit

Permalink
Merge pull request #8527 from OpenMined/feature/add_sender_parameter
Browse files Browse the repository at this point in the history
ADD Email Sender Parameter
  • Loading branch information
jcardonnet authored Feb 26, 2024
2 parents 32a8d93 + 15d62ff commit c723714
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 27 deletions.
1 change: 1 addition & 0 deletions packages/grid/backend/grid/core/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ def get_emails_enabled(cls, v: bool, values: Dict[str, Any]) -> bool:
CONSUMER_SERVICE_NAME: Optional[str] = os.getenv("CONSUMER_SERVICE_NAME")
INMEMORY_WORKERS: bool = str_to_bool(os.getenv("INMEMORY_WORKERS", True))
SMTP_USERNAME: str = os.getenv("SMTP_USERNAME", "")
EMAIL_SENDER: str = os.getenv("EMAIL_SENDER", "")
SMTP_PASSWORD: str = os.getenv("SMTP_PASSWORD", "")

TEST_MODE: bool = (
Expand Down
1 change: 1 addition & 0 deletions packages/grid/backend/grid/core/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,5 @@ def seaweedfs_config() -> SeaweedFSConfig:
in_memory_workers=settings.INMEMORY_WORKERS,
smtp_username=settings.SMTP_USERNAME,
smtp_password=settings.SMTP_PASSWORD,
email_sender=settings.EMAIL_SENDER,
)
5 changes: 2 additions & 3 deletions packages/grid/default.env
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ DEFAULT_ROOT_PASSWORD=changethis
SMTP_TLS=True
SMTP_PORT=587
SMTP_HOST=
SMTP_USER=
SMTP_USERNAME=
SMTP_PASSWORD=
EMAILS_FROM_EMAIL=info@openmined.org
EMAIL_SENDER=
SERVER_HOST="https://${DOMAIN}"
NETWORK_CHECK_INTERVAL=60
DOMAIN_CHECK_INTERVAL=60
Expand All @@ -56,7 +56,6 @@ QUEUE_PORT=5556
CREATE_PRODUCER=False
N_CONSUMERS=1
INMEMORY_WORKERS=True
EMAIL_TOKEN=""

# New Service Flag
USE_NEW_SERVICE=False
Expand Down
8 changes: 6 additions & 2 deletions packages/grid/helm/syft/templates/backend-statefulset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,14 @@ spec:
value: "true"
- name: N_CONSUMERS
value: "0"
- name: SMTP_USERNAME
value: {{ .Values.node.settings.smtpUserName }}
- name: SMTP_PASSWORD
value: {{ .Values.node.settings.smtpPassword }}
- name: EMAIL_SENDER
value: {{ .Values.node.settings.smtpSender }}
- name: INMEMORY_WORKERS
value: "{{ .Values.node.settings.inMemoryWorkers }}"
- name: EMAIL_TOKEN
value: {{ .Values.node.settings.emailToken }}
- name: LOG_LEVEL
value: {{ .Values.node.settings.logLevel }}
- name: DEFAULT_WORKER_POOL_IMAGE
Expand Down
3 changes: 3 additions & 0 deletions packages/grid/helm/syft/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ node:
versionHash: "abc"
nodeSideType: "high"
defaultRootEmail: "info@openmined.org"
smtpUserName: "apikey"
smtpPassword: "password"
smtpSender: "info@openmined.org"
logLevel: "info"
inMemoryWorkers: false
defaultWorkerPoolCount: 1
Expand Down
10 changes: 10 additions & 0 deletions packages/hagrid/hagrid/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,13 @@ def clean(location: str) -> None:
type=str,
help="Password used to auth in email server and enable notification via emails",
)
@click.option(
"--smtp-sender",
default=None,
required=False,
type=str,
help="Sender email used to deliver PyGrid email notifications.",
)
@click.option(
"--build-src",
default=DEFAULT_BRANCH,
Expand Down Expand Up @@ -1325,6 +1332,7 @@ def create_launch_cmd(

parsed_kwargs["smtp_username"] = kwargs["smtp_username"]
parsed_kwargs["smtp_password"] = kwargs["smtp_password"]
parsed_kwargs["smtp_sender"] = kwargs["smtp_sender"]

parsed_kwargs["enable_warnings"] = not kwargs["no_warnings"]

Expand Down Expand Up @@ -2174,6 +2182,7 @@ def create_launch_docker_cmd(
single_container_mode = kwargs["deployment_type"] == "single_container"
in_mem_workers = kwargs.get("in_mem_workers")
smtp_username = kwargs.get("smtp_username")
smtp_sender = kwargs.get("smtp_sender")
smtp_password = kwargs.get("smtp_password")

enable_oblv = bool(kwargs["oblv"])
Expand Down Expand Up @@ -2241,6 +2250,7 @@ def create_launch_docker_cmd(
"INMEMORY_WORKERS": in_mem_workers,
"SMTP_USERNAME": smtp_username,
"SMTP_PASSWORD": smtp_password,
"EMAIL_SENDER": smtp_sender,
}

if "trace" in kwargs and kwargs["trace"] is True:
Expand Down
7 changes: 5 additions & 2 deletions packages/syft/src/syft/node/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ def __init__(
in_memory_workers: bool = True,
smtp_username: Optional[str] = None,
smtp_password: Optional[str] = None,
email_token: Optional[str] = None,
email_sender: Optional[str] = None,
):
# 🟡 TODO 22: change our ENV variable format and default init args to make this
# less horrible or add some convenience functions
Expand Down Expand Up @@ -400,7 +400,10 @@ def __init__(
)

NotifierService.init_notifier(
node=self, email_password=smtp_password, email_username=smtp_username
node=self,
email_password=smtp_password,
email_username=smtp_username,
email_sender=email_sender,
)

self.client_cache = {}
Expand Down
32 changes: 14 additions & 18 deletions packages/syft/src/syft/service/notifier/notifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from .notifier_enums import NOTIFIERS
from .smtp_client import SMTPClient

DEFAULT_EMAIL_SERVER = "smtp.postmarkapp.com"
DEFAULT_EMAIL_SERVER = "smtp.sendgrid.net"


class BaseNotifier:
Expand All @@ -38,27 +38,22 @@ def send(

class EmailNotifier(BaseNotifier):
smtp_client = SMTPClient
username: str
password: str
server: str
port: int
sender = ""

def __init__(
self,
username: str,
password: str,
sender: str,
server: str = DEFAULT_EMAIL_SERVER,
port: int = 587,
) -> None:
self.username = username
self.password = password
self.server = server
self.port = port
self.sender = sender
self.smtp_client = SMTPClient(
server=self.server,
port=self.port,
username=self.username,
password=self.password,
server=server,
port=port,
username=username,
password=password,
)

@classmethod
Expand All @@ -81,9 +76,7 @@ def send(
) -> Result[Ok, Err]:
try:
user_service = context.node.get_service("userservice")
sender_email = user_service.get_by_verify_key(
notification.from_user_verify_key
).email

receiver_email = user_service.get_by_verify_key(
notification.to_user_verify_key
).email
Expand All @@ -97,7 +90,7 @@ def send(
receiver_email = [receiver_email]

self.smtp_client.send(
sender=sender_email, receiver=receiver_email, subject=subject, body=body
sender=self.sender, receiver=receiver_email, subject=subject, body=body
)
return Ok("Email sent successfully!")
except Exception:
Expand Down Expand Up @@ -131,6 +124,7 @@ class NotifierSettings(SyftObject):
NOTIFIERS.APP: False,
}

email_sender: Optional[str] = ""
email_server: Optional[str] = DEFAULT_EMAIL_SERVER
email_port: Optional[int] = 587
email_username: Optional[str] = ""
Expand Down Expand Up @@ -201,7 +195,9 @@ def select_notifiers(self, notification: Notification) -> List[BaseNotifier]:
if notifier_type == NOTIFIERS.EMAIL:
notifier_objs.append(
self.notifiers[notifier_type](
username=self.email_username, password=self.email_password
username=self.email_username,
password=self.email_password,
sender=self.email_sender,
)
)
# If notifier is not email, we just create the notifier object
Expand Down
17 changes: 15 additions & 2 deletions packages/syft/src/syft/service/notifier/notifier_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,14 @@ def turn_on(
context: AuthedServiceContext,
email_username: Optional[str] = None,
email_password: Optional[str] = None,
email_sender: Optional[str] = None,
) -> Union[SyftSuccess, SyftError]:
"""Turn on email notifications.
Args:
email_username (Optional[str]): Email server username. Defaults to None.
email_password (Optional[str]): Email email server password. Defaults to None.
sender_email (Optional[str]): Email sender email. Defaults to None.
Returns:
Union[SyftSuccess, SyftError]: A union type representing the success or error response.
Expand Down Expand Up @@ -108,13 +109,22 @@ def turn_on(
username=email_username, password=email_password
)

if not email_sender and not notifier.email_sender:
return SyftError(
message="You must provide a sender email address to enable notifications."
)

if validation_result.is_err():
return SyftError(
message="Invalid SMTP credentials. Please check your username and password."
)

notifier.email_password = email_password
notifier.email_username = email_username

if email_sender:
notifier.email_sender = email_sender

notifier.active = True
print(
"[LOG] Email credentials are valid. Updating the notifier settings in the db."
Expand Down Expand Up @@ -223,6 +233,7 @@ def init_notifier(
node: AbstractNode,
email_username: Optional[str] = None,
email_password: Optional[str] = None,
email_sender: Optional[str] = None,
) -> Result[Ok, Err]:
"""Initialize Notifier settings for a Node.
If settings already exist, it will use the existing one.
Expand Down Expand Up @@ -259,11 +270,13 @@ def init_notifier(
username=email_username, password=email_password
)

if validation_result.is_err():
sender_not_set = not email_sender and not notifier.email_sender
if validation_result.is_err() or sender_not_set:
notifier.active = False
else:
notifier.email_password = email_password
notifier.email_username = email_username
notifier.email_sender = email_sender
notifier.active = True

notifier_stash.set(node.signing_key.verify_key, notifier)
Expand Down
2 changes: 2 additions & 0 deletions packages/syft/src/syft/service/settings/settings_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,14 @@ def enable_notifications(
context: AuthedServiceContext,
email_username: Optional[str] = None,
email_password: Optional[str] = None,
email_sender: Optional[str] = None,
) -> Union[SyftSuccess, SyftError]:
notifier_service = context.node.get_service("notifierservice")
return notifier_service.turn_on(
context=context,
email_username=email_username,
email_password=email_password,
email_sender=email_sender,
)

@service_method(
Expand Down

0 comments on commit c723714

Please sign in to comment.