search

ietf-tools / mailarchive

IETF Mail List Archives
https://mailarchive.ietf.org
BSD 3-Clause "New" or "Revised" License
42 stars 25 forks source link

refactor: change message import API to use JSON payload #3773

Closed rpcross closed 3 months ago

rpcross commented 3 months ago

I have completed the parameter changes discussed earlier for this PR

rjsparks commented 3 months ago

I'm not sure I'm ok with sending a 500 back up the stack intentionally. Won't that trigger email to the admins with a stackdump? Or am I missing something?

jennifer-richards commented 3 months ago

I'm not sure I'm ok with sending a 500 back up the stack intentionally. Won't that trigger email to the admins with a stackdump? Or am I missing something?

I'm not sure I'm ok with sending a 500 back up the stack intentionally. Won't that trigger email to the admins with a stackdump? Or am I missing something?

Just checked and it will send the admin crash email on any 5xx response code.

I think this is the right thing to do for the first case I marked. If it gets an OSError, FileNotFoundError, or PermissionError then something is broken on the server and the admins should be aware.

Contrary to my suggestion, I agree 400 is fine for the other one. From the method names I'd thought the archive_message() method was just saving an already-validated message, but it seems it's doing more validation in there.

rpcross commented 3 months ago

@jennifer-richards I have made the requested changes. Please review them when you have a chance.