1. PRs generally aren't "approved" in Github if a core contributor looks at it and is happy 2. It was merged by criccomini, not the opener https://github.com/apache/incubator-airflow/commit/7cb818bbacb2a2695282471591a9e323d8efbf5c <https://github.com/apache/incubator-airflow/commit/7cb818bbacb2a2695282471591a9e323d8efbf5c> 3, 4, 5. I agree, and I made the same point https://issues.apache.org/jira/browse/AIRFLOW-1723 <https://issues.apache.org/jira/browse/AIRFLOW-1723>
Either way this is a bug that it always tries to import unconditionally and fails when the sendgrid mailer isn't used. > On 25 Oct 2017, at 09:48, Sumit Maheshwari <[email protected]> wrote: > > 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 >> >>
