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

added binding AWS SES #1021

Merged
merged 18 commits into from
Aug 14, 2021
Merged

added binding AWS SES #1021

merged 18 commits into from
Aug 14, 2021

Conversation

fjvela
Copy link
Contributor

@fjvela fjvela commented Jul 18, 2021

Description

New binding AWS SES

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1019

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

image

@fjvela fjvela changed the title added binding AWS SES WIP: added binding AWS SES Jul 18, 2021
@fjvela fjvela marked this pull request as ready for review July 19, 2021 14:26
@fjvela fjvela requested review from a team as code owners July 19, 2021 14:26
@fjvela fjvela changed the title WIP: added binding AWS SES added binding AWS SES Jul 19, 2021
bindings/aws/ses/ses.go Outdated Show resolved Hide resolved
// Create an SES instance
svc := ses.New(sess)

body, err := strconv.Unquote(string(req.Data))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why unquote? Is this a JSON String? If so, use JSON deserializer instead, so it can handle escaped quotes inside the string correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In SMTP and sendgrid binding it's used Unquoted method.

Must I change it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@artursouza did you see my comment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked and Unquote() will handle properly escaped quotes too: https://play.golang.org/p/LQHgvD9nZil

bindings/aws/ses/ses.go Outdated Show resolved Hide resolved
@artursouza artursouza self-assigned this Aug 4, 2021
@yaron2
Copy link
Member

yaron2 commented Aug 6, 2021

@fjvela ping

@codecov
Copy link

codecov bot commented Aug 14, 2021

Codecov Report

Merging #1021 (1bf87d0) into master (253ef85) will decrease coverage by 0.40%.
The diff coverage is 24.69%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1021      +/-   ##
==========================================
- Coverage   34.53%   34.12%   -0.41%     
==========================================
  Files         132      134       +2     
  Lines       10870    11146     +276     
==========================================
+ Hits         3754     3804      +50     
- Misses       6736     6950     +214     
- Partials      380      392      +12     
Impacted Files Coverage Δ
authentication/azure/storage.go 0.00% <0.00%> (ø)
bindings/smtp/smtp.go 53.33% <0.00%> (-1.84%) ⬇️
pubsub/rocketmq/rocketmq.go 0.00% <ø> (ø)
pubsub/rocketmq/settings.go 86.66% <ø> (ø)
state/azure/tablestorage/tablestorage.go 13.20% <0.00%> (ø)
bindings/alicloud/tablestore/tablestore.go 2.54% <2.54%> (ø)
bindings/aws/ses/ses.go 24.05% <24.05%> (ø)
state/redis/redis.go 42.22% <33.33%> (-0.64%) ⬇️
state/azure/blobstorage/blobstorage.go 24.46% <50.00%> (+0.01%) ⬆️
authentication/azure/auth.go 57.02% <57.02%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7aac0ab...1bf87d0. Read the comment docs.

@dapr-bot dapr-bot merged commit 405ba37 into dapr:master Aug 14, 2021
@fjvela fjvela deleted the binding-aws-ses branch August 14, 2021 02:26
amimimor pushed a commit to amimimor/components-contrib that referenced this pull request Dec 9, 2021
* added binding AWS SES

* binding AWS SES: removed unused code

* binding AWS SES: fix lint

* binding AWS SES: fix parse session token

* binding AWS SES: support multiple email && fix cc / bcc parse error

* binding AWS S3: add TODO configuration set

* binding AWS SES: handle error on unquote req.Data

* binding AWS SES: fix lint errors

* binding AWS SES: reuse SES instance

Co-authored-by: Artur Souza <artursouza.ms@outlook.com>
Co-authored-by: Yaron Schneider <yaronsc@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New binding AWS SES
5 participants