Hi Sumit, Thanks for pointing out. I also stumbled into the same problem of missing the sendgrid dependancy on my development environment. My suggestion would be to revert the commit for now and reimplement it without relying on a hard dependancy.
As discussed in the Jira, this should be implemented as a plugin. I would suggest a similar structure as we do with the logging handlers: https://github.com/apache/incubator-airflow/tree/master/airflow/utils/log Cheers, Fokko 2017-10-25 8:48 GMT+00:00 Sumit Maheshwari <[email protected]>: > Sorry, mistakenly sent halfbaked: > > So, the concerns are: > > 1. Don't see any approver on the PR, neither +1s. > 2. The PR author and PR merging guy seem to be same, which I think is > against the general understanding. > 3. Why sendgrid got special privileges, and why not 100s of other mail > services? The Same concern was raised by Ash in JIRA as well. > 4. Why it was not coded as a plugin. > 5. Why there is a hard dependency to install Sendgrid module if I am not > using it. > > I think this commit needs to be reverted and a new PR should be raised, > which adds its as a plugin instead of a core feature. > > > > On Wed, Oct 25, 2017 at 2:12 PM, Sumit Maheshwari <[email protected]> > wrote: > > > When I fetched the latest master, installed it and ran webserver, it > > failed with this error: > > > > *ImportError: No module named sendgrid* > > > > On further investigation, I found that it has been introduced as part of > > this PR <https://github.com/apache/incubator-airflow/pull/2695> and this > > JIRA <https://issues.apache.org/jira/browse/AIRFLOW-1723>. > > Now couple of doubts: > > > > 1. > > > > > > > > > > Thanks, > > Sumit Maheshwari > > cell. 9632202950 > > > > >
