Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add register and token actions, improve message content and format #13

Merged
merged 3 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ cryptography = "^43.0.3"
PyYAML = "^6.0.0"
npg_porch_cli = { git="https://github.com/wtsi-npg/npg_porch_cli.git", tag="0.1.0" }
partisan = { url = "https://github.com/wtsi-npg/partisan/releases/download/2.13.0/partisan-2.13.0.tar.gz" }
npg-python-lib = { url = "https://github.com/wtsi-npg/npg-python-lib/releases/download/0.3.2/npg_python_lib-0.3.2.tar.gz" }
npg-python-lib = { url = "https://github.com/wtsi-npg/npg-python-lib/releases/download/0.3.4/npg_python_lib-0.3.4.tar.gz" }
requests = "^2.32.0"
structlog = "^24.4.0"

Expand Down
10 changes: 6 additions & 4 deletions src/npg_notify/data/resources/ont_event_email_template.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
The ONT run for experiment $experiment_name, flowcell $flowcell_id has been $event.
The data are available in iRODS at the following path:
The ONT run for experiment $experiment_name, flowcell $flowcell_id has been $event. The data are available in iRODS at the following path:

$path

This is an automated email from NPG. You are receiving it because you are registered
as a contact for one or more of the Studies listed below:
This is an automated email from NPG. You are receiving it because you are registered as a contact for one or more of the Studies listed below:

$studies

If you have any questions or need further assistance, please feel free to contact a Scientific Service Representative at dnap-ssr@$domain.

NPG on behalf of DNA Pipelines.
90 changes: 69 additions & 21 deletions src/npg_notify/ont/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from enum import Enum
from importlib import resources
from string import Template
from typing import Type

from npg.cli import add_io_arguments, add_logging_arguments
from npg.conf import IniData
Expand Down Expand Up @@ -79,6 +80,9 @@
user = <MySQL user>
password = <MySQL password>
db = <MySQL database>

[MAIL]
domain = <email FQDN>
"""


Expand Down Expand Up @@ -167,15 +171,23 @@ def subject(self) -> str:
f"has been {self.event}"
)

def body(self, *study_ids: list[str]) -> str:
def body(self, studies: list[Study], domain: str = None) -> str:
"""Return the body of the email.

Args:
*study_ids: The study IDs associated with the run.
studies: The studies associated with the run.
domain: A network domain name to use when sending email. The main will
be sent from mail.<domain> with <user>@<domain> in the From: header.
"""
if domain is None:
raise ValueError("domain is required")

source = resources.files("npg_notify.data.resources").joinpath(
"ont_event_email_template.txt"
)

study_descs = [f"{s.id_study_lims} ({s.name})" for s in studies]

with resources.as_file(source) as template:
with open(template) as f:
t = Template(f.read())
Expand All @@ -185,7 +197,8 @@ def body(self, *study_ids: list[str]) -> str:
"flowcell_id": self.flowcell_id,
"path": self.path,
"event": self.event,
"studies": "\n".join([*study_ids]),
"studies": "\n".join(study_descs),
"domain": domain,
}
)

Expand All @@ -202,6 +215,14 @@ def to_serializable(self) -> dict:
def from_serializable(cls, serializable: dict):
return cls(**serializable)

def __str__(self):
return (
f"<ONT experiment: {self.experiment_name} "
f"instrument slot: {self.instrument_slot} "
f"flowcell ID: {self.flowcell_id} "
f"event: {self.event}>"
)


def add_email_tasks(
pipeline: Pipeline, event: EventType, reader, writer
Expand Down Expand Up @@ -285,22 +306,30 @@ def run_email_tasks(
for task in pipeline.claim(batch_size):
try:
np += 1
study_ids = find_studies_for_run(
studies = find_studies_for_run(
session, task.experiment_name, task.instrument_slot, task.flowcell_id
)

# We are sending a single email to all contacts of all studies in the run
contacts = set()
for study_id in study_ids:
c = get_study_contacts(session=session, study_id=study_id)
for study in studies:
c = get_study_contacts(session=session, study_id=study.id_study_lims)
contacts.update(c)

log.info(
"Preparing email",
pipeline=pipeline,
task=task,
studies=[s.id_study_lims for s in studies],
contacts=contacts,
)

if len(contacts) == 0:
log.info(
"No contacts found",
pipeline=pipeline,
task=task,
study_ids=study_ids,
studies=studies,
)

pipeline.done(task)
Expand All @@ -312,7 +341,7 @@ def run_email_tasks(
domain=domain,
contacts=sorted(contacts),
subject=task.subject(),
content=task.body(study_ids),
content=task.body(studies),
)

pipeline.done(task)
Expand Down Expand Up @@ -341,8 +370,9 @@ def run_email_tasks(

def find_studies_for_run(
session: Session, experiment_name: str, instrument_slot: int, flowcell_id: str
) -> list[str]:
"""Return the study IDs associated with an ONT run.
) -> list[Type[Study]]:
"""Return the studies associated with an ONT run, ordered by ascending
id_study_lims.

Args:
session: An open MLWH DB session.
Expand All @@ -351,11 +381,10 @@ def find_studies_for_run(
flowcell_id: The flowcell ID.

Returns:
Study IDs associated with the run.
Studies associated with the run.
"""
return [
elt
for elt, in session.query(distinct(Study.id_study_lims))
return (
session.query(Study)
.join(OseqFlowcell)
.filter(
OseqFlowcell.experiment_name == experiment_name,
Expand All @@ -364,7 +393,13 @@ def find_studies_for_run(
)
.order_by(asc(Study.id_study_lims))
.all()
]
)


@dataclass
class EmailConfig:
domain: str
"""The domain name to use when sending email. The main will be sent from mail.<domain>"""


def main():
Expand All @@ -378,9 +413,15 @@ def main():
help="The 'add' action acts as a producer by sending new notification "
"tasks to the Porch server. The 'run' action acts as a consumer "
"by retrieving any notification tasks that have not been done and "
"running them to send notifications.",
choices=["add", "run"],
# nargs="?",
"running them to send notifications."
""
"The remaining actions are for administrative purposes and require an admin "
"token to be set in the configuration file."
"The 'register' action registers the pipeline with the Porch server. It must "
"be run once, before any tasks can be added or run. The 'token' action "
"generates a new token for the pipeline. This token is used to authenticate "
"the pipeline with the Porch server.",
choices=["add", "run", "register", "token"],
)
parser.add_argument(
"--event",
Expand Down Expand Up @@ -416,14 +457,15 @@ def main():
)

config_file = args.conf_file_path
config = IniData(Pipeline.ServerConfig).from_file(config_file, "PORCH")
pipeline_config = IniData(Pipeline.ServerConfig).from_file(config_file, "PORCH")
email_config = IniData(EmailConfig).from_file(config_file, "MAIL")

pipeline = Pipeline(
ContactEmail,
name="ont-event-email",
uri="https://github.com/wtsi/npg_notifications.git",
version=npg_notify.version(),
config=config,
config=pipeline_config,
)

num_processed, num_succeeded, num_errors = 0, 0, 0
Expand All @@ -437,8 +479,14 @@ def main():
elif action == "run":
with get_connection(config_file, MYSQL_MLWH_CONFIG_FILE_SECTION) as session:
num_processed, num_succeeded, num_errors = run_email_tasks(
pipeline, session
pipeline, session, email_config.domain
)
elif action == "register":
pipeline.register()
elif action == "token":
print(pipeline.new_token("ont-event-email"))
else:
raise ValueError(f"Unknown action: {action}")

if num_errors > 0:
log.error(
Expand Down
20 changes: 13 additions & 7 deletions tests/ont/test_generate_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from pytest import mark as m

from npg_notify.db.mlwh import Study
from npg_notify.ont.event import ContactEmail, EventType


Expand Down Expand Up @@ -43,7 +44,9 @@ def test_generate_email(self):
flowcell_id = "FAKE12345"
path = f"/testZone/home/irods/{expt}_{slot}_{flowcell_id}"
event_type = EventType.UPLOADED
studies = ["study1", "study2"]

domain = "no-such-domain.sanger.ac.uk"
studies = [Study(name="study1"), Study(name="study2")]

event = ContactEmail(
experiment_name=expt,
Expand All @@ -58,16 +61,19 @@ def test_generate_email(self):
== f"Update: ONT run {expt} flowcell {flowcell_id} has been {event_type}"
)

study_lines = "\n".join(studies)
study_descs = [f"{s.id_study_lims} ({s.name})" for s in studies]
study_lines = "\n".join(study_descs)

assert event.body(*studies) == (
f"The ONT run for experiment {expt}, flowcell {flowcell_id} has been {event_type}.\n"
"The data are available in iRODS at the following path:\n"
assert event.body(studies, domain=domain) == (
f"The ONT run for experiment {expt}, flowcell {flowcell_id} has been {event_type}. The data are available in iRODS at the following path:\n"
"\n"
f"{path}\n"
"\n"
"This is an automated email from NPG. You are receiving it because you are registered\n"
"as a contact for one or more of the Studies listed below:\n"
"This is an automated email from NPG. You are receiving it because you are registered as a contact for one or more of the Studies listed below:\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notifications for Illumina and PacBio platforms have a standard footer. Do we really want ONT notifications to be different?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one?

"If you have any questions or need further assistance, please feel "
+ "free to reach out to a Scientific Service Representative at "
+ f"dnap-ssr@{domain}.",
"",
"NPG on behalf of DNA Pipelines\n",

Yes, it's good for it to be the same.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Lines 152-154 were added on request by SSRs

Copy link
Member Author

@kjsanger kjsanger Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also expecting to add a few other details in the week or two after it's live - listing the plate barcodes and tag names/sequences.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Footer added

"\n"
f"{study_lines}\n"
"\n"
f"If you have any questions or need further assistance, please feel free to contact a Scientific Service Representative at dnap-ssr@{domain}.\n"
"\n"
"NPG on behalf of DNA Pipelines.\n"
)