search

serilog / serilog-sinks-email

A Serilog sink that writes events to SMTP email
Apache License 2.0
72 stars 68 forks source link

Open MailKit secure socket option setting #87

Closed arthurchiakh closed 3 years ago

arthurchiakh commented 3 years ago

related to Issue #69

Enable user to set secure socket option for MailKit

arthurchiakh commented 3 years ago

Hi, @nblumhardt , do you mind checking this PR? This change is to let user to specify secure socket option setting for MailKit, which can increase the range of supported email servers for this library. In my case, I can only connect to Office365 email server when the secure socket option set to SecureSocketOptions.StartTls. Please see also related issue #69 Mailkit client use SecureSocketOptions .

Gondost commented 3 years ago

I would like to see this merged as well, I need to set the SecureSocketOptions in order to send email in my environment.

nblumhardt commented 3 years ago

Thanks for the nudge. I would prefer not to expose MailKit types directly on the email sink's API - is there a higher-level way this can be expressed, akin to how we currently set EnableSSL?

Gondost commented 3 years ago

@nblumhardt

EnableSSL is a bit simpler because it's a boolean on both ends, whereas SecureSocketOptions is an enum. At some point, the enum will have to be validated to ascertain whether it's valid. This seems to lead to paths like creating your own enum with the same values as SecureSocketOptions, etc. Or, casting it in some manner and doing an automatic mapping. One way or the other, enum/type mapping seems unavoidable, so it seems like it's a matter of how you prefer to structure the sink code.

andriysavin commented 3 years ago

Thanks for the nudge. I would prefer not to expose MailKit types directly on the email sink's API - is there a higher-level way this can be expressed, akin to how we currently set EnableSSL?

I think this sink should introduce an enum similar or the same to SecureSocketOptions to avoid dependency on MailKit. And have one-to-one mapping between these enums. Come on, guys, lets finish this, it will be very useful!

nblumhardt commented 3 years ago

Introducing an enum that maps to MailKit's one sounds reasonable 👍

arthurchiakh commented 3 years ago

I'm sorry guys, better late than never. I have added a dedicated Enum to map the MailKit socket options.

@nblumhardt please review my latest change. Thanks loads.

arthurchiakh commented 3 years ago

@nblumhardt @andriysavin I already changed the code accordingly. Please let me know anything that I can improve. Thanks.

arthurchiakh commented 3 years ago

@nblumhardt Hi, can you have a look on this?

andriysavin commented 3 years ago

@nblumhardt please, take a look

nblumhardt commented 3 years ago

Righto! 👍 Let's give this a try on dev 😎

andriysavin commented 3 years ago

@nblumhardt Is there a package feed for dev branch?

arthurchiakh commented 3 years ago

@nblumhardt Is there a package feed for dev branch?

I believe this is the one. https://www.nuget.org/packages/Serilog.Sinks.Email/2.4.1-dev-00128

lenlenya commented 3 years ago

Thank you for your contribution, It's very helpful to me.

chekm8 commented 2 years ago

Thanks for this, The pre-release code resolves my issue as well. Any idea when the final release will be published?