Skip to content
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

Mail helper docstring is wrong #569

Closed
onecrayon opened this issue Apr 13, 2018 · 1 comment · Fixed by #570
Closed

Mail helper docstring is wrong #569

onecrayon opened this issue Apr 13, 2018 · 1 comment · Fixed by #570
Labels
difficulty: unknown or n/a fix is unknown in difficulty status: help wanted requesting help from the community type: bug bug in the library

Comments

@onecrayon
Copy link
Contributor

Issue Summary

The Mail helper docstring claims that all of its arguments are optional. However, if you fail to pass one of them then none of them are saved (it's all or nothing):

https://github.com/sendgrid/sendgrid-python/blob/master/sendgrid/helpers/mail/mail.py#L44

This was a major annoyance to debug (kept getting 400 Bad Request errors because the "from" and "to" emails were getting silently discarded since I was using explicit add_content calls to populate both plain text and HTML content after initializing the Mail object).

My preference would be to have anything passed into the Mail helper populate its internal variables, thus making the docstring correct. Alternately, adjusting the docstring to make it clear the arguments are all or nothing would be very much appreciated.

Technical details:

  • sendgrid-python Version: 5.3.0
  • Python Version: 3.6.5
@thinkingserious thinkingserious added type: community enhancement feature request not on Twilio's roadmap status: help wanted requesting help from the community difficulty: unknown or n/a fix is unknown in difficulty type: bug bug in the library and removed type: community enhancement feature request not on Twilio's roadmap labels Apr 13, 2018
@thinkingserious
Copy link
Contributor

Hi @onecrayon,

Thanks for the heads up! In the short term, I think we should fix the docstrings. I'm adding this to our backlog, but feel free to submit a PR if you like.

In the next version, we will make these optional. If you have any feedback on the next version, we would love to have it :)

With Best Regards,

Elmer

onecrayon added a commit to onecrayon/sendgrid-python that referenced this issue Apr 16, 2018
Closes sendgrid#569.

This is a stop-gap measure prior to the Mail helper being rewritten for a future update; the API ultimately returns errors if an element isn't specified regardless, so this makes the initialization a little more flexible (allows specifying a `to_email` at initialization to take advantage of automatic Personalization creation while still specifying the content later, for instance).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: unknown or n/a fix is unknown in difficulty status: help wanted requesting help from the community type: bug bug in the library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants