[AIRFLOW-1867] Fix sendgrid py3k bug; add sandbox mode #2824
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thesquelched
force-pushed
the
AIRFLOW-1867
branch
from
09a5550 to
52fc5d6
Compare
Codecov Report
@@ Coverage Diff @@
## master #2824 +/- ##
==========================================
- Coverage 75.91% 75.91% -0.01%
==========================================
Files 199 199
Lines 15959 15957 -2
==========================================
- Hits 12116 12113 -3
- Misses 3843 3844 +1
Continue to review full report at Codecov.
|
gwax
reviewed
Jan 10, 2018
tests/contrib/utils/test_sendgrid.py
Outdated
|
|
||
| # Test the right email is constructed. | ||
| @mock.patch('os.environ.get') | ||
| @mock.patch('airflow.contrib.utils.sendgrid._post_sendgrid_mail') | ||
| def test_send_email_sendgrid_correct_email(self, mock_post, mock_get): | ||
| mock_get.return_value = '[email protected]' | ||
| send_email(self.to, self.subject, self.html_content, cc=self.cc, bcc=self.bcc) | ||
|
|
||
| tempdir = tempfile.mkdtemp() |
There was a problem hiding this comment.
Since we only need one file, can we use tempfile.NamedTemporaryFile in a context instead of dealing with a temporary directory (and removing it)?
with tempfile.NamedTemporaryFile(mode='wt') as f:
f.write('this is some test data')
f.flush()
send_email(..., files=[f.name])
airflow/contrib/utils/sendgrid.py
Outdated
| attachment.filename = basename | ||
| attachment.disposition = "attachment" | ||
| attachment.content_id = '<%s>' % basename | ||
| attachment.content = base64.b64encode(f.read()).decode('utf8') |
There was a problem hiding this comment.
Are we sure that the sendgrid api expects a str and not a bytes?
There was a problem hiding this comment.
Yes. The sendgrid code turns this data into a python dict and then json.dumps it. In python3, bytes can not be JSON-serialized:
Python 3.4.2 (default, Oct 8 2014, 10:45:20)
[GCC 4.9.1] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import json
>>> json.dumps(b'welkrjeklwj')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/python3.4/json/__init__.py", line 230, in dumps
return _default_encoder.encode(obj)
File "/usr/lib/python3.4/json/encoder.py", line 192, in encode
chunks = self.iterencode(o, _one_shot=True)
File "/usr/lib/python3.4/json/encoder.py", line 250, in iterencode
return _iterencode(o, 0)
File "/usr/lib/python3.4/json/encoder.py", line 173, in default
raise TypeError(repr(o) + " is not JSON serializable")
TypeError: b'welkrjeklwj' is not JSON serializable| attachment = Attachment() | ||
| attachment.type = mimetypes.guess_type(basename)[0] |
There was a problem hiding this comment.
Thanks for pulling out everything that doesn't need to be in the context.
thesquelched
force-pushed
the
AIRFLOW-1867
branch
from
52fc5d6 to
c20bb53
Compare
|
Please fix the CI |
thesquelched
force-pushed
the
AIRFLOW-1867
branch
from
c20bb53 to
52d1026
Compare
|
@thesquelched Please fix the CI |
thesquelched
force-pushed
the
AIRFLOW-1867
branch
from
52d1026 to
62c53aa
Compare
- Fix sendgrid attachments bug in py3k - Add sandbox mode option - Minor style fixes - Add test
thesquelched
force-pushed
the
AIRFLOW-1867
branch
from
c1a3c9c to
123cf66
Compare
|
@Fokko fixed CI failures, rebased, and squashed |
|
Thanks @thesquelched |
tekn0ir
pushed a commit
to tekn0ir/incubator-airflow
that referenced
this pull request
Oct 26, 2018
* master: [AIRFLOW-520] Fix Version Info in Flask UI (apache#4072) [AIRFLOW-XXX] Add Neoway to companies list (apache#4081) [AIRFLOW-XXX] Add Surfline to companies list (apache#4079) Revert "[AIRFLOW-461] Restore parameter position for BQ run_load method (apache#4077)" [AIRFLOW-461] Restore parameter position for BQ run_load method (apache#4077) [AIRFLOW-461] Support autodetected schemas in BigQuery run_load (apache#3880) [AIRFLOW-3238] Fix models.DAG to deactivate unknown DAGs on initdb (apache#4073) [AIRFLOW-3239] Fix test recovery further (apache#4074) [AIRFLOW-3203] Fix DockerOperator & some operator test (apache#4049) [AIRFLOW-1867] Add sandbox mode and py3k bug (apache#2824) [AIRFLOW-2993] s3_to_sftp and sftp_to_s3 operators (apache#3828) [AIRFLOW-XXX] BigQuery Hook - Minor Refactoring (apache#4066) [AIRFLOW-3232] More readable GCF operator documentation (apache#4067)
aliceabe
pushed a commit
to aliceabe/incubator-airflow
that referenced
this pull request
Jan 3, 2019
- Fix sendgrid attachments bug in py3k - Add sandbox mode option - Minor style fixes - Add test
wmorris75
pushed a commit
to modmed/incubator-airflow
that referenced
this pull request
Jul 29, 2019
- Fix sendgrid attachments bug in py3k - Add sandbox mode option - Minor style fixes - Add test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dear Airflow maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
JIRA
Description
Tests
tests/contrib/utils/test_sendgrid.py: add attachment to existing testCommits