-
Notifications
You must be signed in to change notification settings - Fork 439
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
Use dataclass for email data #9365
base: main
Are you sure you want to change the base?
Conversation
fdfecfa
to
27dd03a
Compare
|
||
:param request: the current request | ||
:type request: pyramid.request.Request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving the types to the signature of the function
tag: EmailTag | ||
html: str | None = None | ||
|
||
def to_message(self) -> pyramid_mailer.message.Message: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incapsulating converting the email data into pyramid mailer message
27dd03a
to
41bfb2c
Compare
|
||
@celery.task(bind=True, max_retries=3, acks_late=True) | ||
|
||
@celery.task(bind=True, max_retries=3, acks_late=True, serializer="pickle") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enabling pickle to support dataclass serialization just for this task
h/tasks/celery.py
Outdated
@@ -19,6 +19,7 @@ | |||
|
|||
celery = Celery("h") | |||
celery.conf.update( | |||
accept_content=['json', 'pickle'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Letting the workers accept pickled dataclass serialization
Not sure what are policy here is but having to stick to json-serializable is kinda limiting.
Alternatively we could write our own serializer / deserializer pair and register it with kombu.serialization
to be more explicit with what we send and receive.
9701aae
to
881700a
Compare
subject: str, | ||
body: str, | ||
tag: EmailTag, | ||
recipients: list[str] | EmailData, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Number of arguments doesn't change but we support both old and new signatures by inspecting the type of the first argument below to support in-flight celery tasks with the old arguments.
881700a
to
adb7578
Compare
Refs #9332
Now that we've added tag to email data it makes sense to incapsulate it all in a separate class and pass it around instead of a tuple.
Testing
devdata_admin:pass
admin@example.com
emailSee Don't send multiple emails on reply with mentions #9356 for instructions
See Don't send multiple emails on reply with mentions #9356 for instructions
devdata_biopub_creator
in biopub open group/mail
folder for new emailsTo test support of both
send
signatures you can run something like this from the cli.