-
-
Notifications
You must be signed in to change notification settings - Fork 612
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
[10.0][FIX] mail_sendgrid_mass_mailing: Bad import #288
Conversation
@ecino can you check? |
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.
Would be great to change error logs by warning. Otherwise the fix is good, thanks.
@@ -7,7 +7,7 @@ | |||
_logger = logging.getLogger(__name__) | |||
|
|||
try: | |||
from sendgrid.helpers.mail.mail import TrackingSettings, \ | |||
from sendgrid.helpers.mail import TrackingSettings, \ | |||
SubscriptionTracking | |||
except ImportError: | |||
_logger.error("ImportError raised while loading module.") |
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.
While you're at it, could you replace all the import error logs by warnings? This will prevent some instances to fail when sendgrid is not installed. Please check other cases in the module as well in mail_sendgrid module. Thanks!
@igallart Did you have the chance to look at my suggestion? |
@ecino, after I try do your suggestions, I don't have very time now, sorry! |
@ecino, correct? |
Perfect, thanks! |
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.
LINT errors are in other modules, for me this pull request is good and should be merged quickly because it fixes errors in the tests.
OK, I have fixed RST syntax problems in 1bd032d, so we can proceed with this. |
[10.0] Migrate sendgrid modules CO-1192 The mail are now correctly sent to all recipients Sendgrid code corrections for v10 + Add tests Rewrite dependencies of sendgrid module fixes OCA#261 [UPD] Update mail_sendgrid.pot [10.0][FIX] mail_sendgrid_mass_mailing: Bad import (OCA#288) [UPD] Update mail_sendgrid.pot [FIX] mail_sendgrid*. Use info loglevel for import error. Using _logger.warning for ImportError exception results in runbot/travis failures. Add indexes to speed up performance [IMP] use json library instead of simplejson migrate mail_sendgrid to v11. Add support of sendgrid v3 API. Add pre-migration script and update readme No more commit in DB when tests are enabled FIX: Ttracking event modify also the mail.mail status FIX multiple body fields that were conflicting - Instead of copying body of template to mass mailing, directly use the template text for mass mailings when using Sendgrid. - If needed, the template must be edited instead of the mass mailing. - This simplifies the code complexity FIX flake8 issue
[10.0] Migrate sendgrid modules CO-1192 The mail are now correctly sent to all recipients Sendgrid code corrections for v10 + Add tests [UPD] Update mail_sendgrid_mass_mailing.pot [10.0][FIX] mail_sendgrid_mass_mailing: Bad import (OCA#288) [FIX] mail_sendgrid*. Use info loglevel for import error. Using _logger.warning for ImportError exception results in runbot/travis failures. Title mistake [UPD] Update mail_sendgrid_mass_mailing.pot [FIX] don't use sendgrid v6.0.0 as the code is made for lower versions FIX multiple body fields that were conflicting - Instead of copying body of template to mass mailing, directly use the template text for mass mailings when using Sendgrid. - If needed, the template must be edited instead of the mass mailing. - This simplifies the code complexity [UPD] Update mail_sendgrid_mass_mailing.pot [MIG] mail_sendgrid_mass_mailing: Migration to 11.0
[10.0] Migrate sendgrid modules CO-1192 The mail are now correctly sent to all recipients Sendgrid code corrections for v10 + Add tests [UPD] Update mail_sendgrid_mass_mailing.pot [10.0][FIX] mail_sendgrid_mass_mailing: Bad import (OCA#288) [FIX] mail_sendgrid*. Use info loglevel for import error. Using _logger.warning for ImportError exception results in runbot/travis failures. Title mistake [UPD] Update mail_sendgrid_mass_mailing.pot [FIX] don't use sendgrid v6.0.0 as the code is made for lower versions FIX multiple body fields that were conflicting - Instead of copying body of template to mass mailing, directly use the template text for mass mailings when using Sendgrid. - If needed, the template must be edited instead of the mass mailing. - This simplifies the code complexity [UPD] Update mail_sendgrid_mass_mailing.pot [MIG] mail_sendgrid_mass_mailing: Migration to 11.0
[10.0] Migrate sendgrid modules CO-1192 The mail are now correctly sent to all recipients Sendgrid code corrections for v10 + Add tests [UPD] Update mail_sendgrid_mass_mailing.pot [10.0][FIX] mail_sendgrid_mass_mailing: Bad import (OCA#288) [FIX] mail_sendgrid*. Use info loglevel for import error. Using _logger.warning for ImportError exception results in runbot/travis failures. Title mistake [UPD] Update mail_sendgrid_mass_mailing.pot [FIX] don't use sendgrid v6.0.0 as the code is made for lower versions FIX multiple body fields that were conflicting - Instead of copying body of template to mass mailing, directly use the template text for mass mailings when using Sendgrid. - If needed, the template must be edited instead of the mass mailing. - This simplifies the code complexity [UPD] Update mail_sendgrid_mass_mailing.pot [MIG] mail_sendgrid_mass_mailing: Migration to 11.0
No description provided.