-
Notifications
You must be signed in to change notification settings - Fork 5
feat/#16/more-robust-attachements-and-report-options #17
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
feat/#16/more-robust-attachements-and-report-options #17
Conversation
… not provided defaults to chrome window env var values) * Added the ability to send PNGs (great for simple tables -- default report will still be PDF) * Added the ability to inline attachments in emails (default is a standard email attachment) * Ran rustfmt on the files I touched * Addressed simple warnings
|
Next week/weekend I will try and setup some testing for this and upload the results. Been busy at work with releases and at home doing spring cleaning. |
# Conflicts: # src/config.rs # src/router.rs
|
Did some manual QA testing and merged main into my branch, let me know if you're looking for any other testing here I anticipate a mild amount of churn on meeting your teams code standards, I'm familiar with rest services but this is my first time working with them in Rust. |
src/router.rs
Outdated
| // You should have received a copy of the GNU Affero General Public License | ||
| // along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| use actix_web::web::Query; |
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.
I think you can remove this
src/lib.rs
Outdated
| pub mod config; | ||
| pub mod router; | ||
|
|
||
| use actix_web::cookie::time::format_description::well_known::iso8601::Config; |
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.
You can remove this
src/lib.rs
Outdated
| message::{header::ContentType, MultiPart, SinglePart}, | ||
| AsyncSmtpTransport, AsyncTransport, Message, Tokio1Executor, | ||
| }; | ||
| use log::Log; |
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.
This is also not used looks like
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.
Good catches, I addressed all the warnings I saw with cargo run and cargo build -r which included these unused imports.
I also added some validation on the send report endpoint to raise some helpful errors
Error for inline PDFs (maybe possible on some email servers, but is not possible in gmail today):

- [enhancement] add error handling for inline PDFs - [enhancement] add error handling for when no dashboards are sent in request for send_report
|
Thanks @Kurloc for the pr 🚀. I will raise a feature request in the main openobserve repo for this. |





Features Added:
This will need a PR in the main O2 repo to address the following items before it use usable by most users: