Closed woutercouvaras closed 2 years ago
Base: 100.00% // Head: 100.00% // No change to project coverage :thumbsup:
Coverage data is based on head (
82f8d43
) compared to base (4db06d2
). Patch coverage: 100.00% of modified lines in pull request are covered.:exclamation: Current head 82f8d43 differs from pull request most recent head 360c773. Consider uploading reports for the commit 360c773 to get more accurate results
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
I'm embarassed to say that I didn't see the tests before :) I'll take a look at them and try to fix the coverage issue, as well as these other issues.
I will reformat the title to use the proper commit message syntax.
@mtrezza let me know if you need me to tackle anything else.
I've pushed the suggested changes. Not sure I should be the one to resolve those, sorry. I should have checked with you first.
Regarding the change to get credentials...
It is a bit odd and I made the same assumption as you when I first tried the new sdk. Either way, I've just tested it again and sadly that doesn't work. fromInstanceMetadata
& fromEnv
return an async function that one needs to call to get the credentials :(
I've just changed it back and pushed.
But the line contains 2 await
, so the async function should be executed:
const credentials = process.env.NODE_ENV == 'production' ? await fromInstanceMetadata() : await fromEnv();
Or is the syntax wrong, maybe the await
has to move to the front?
Sorry, it seems I hadn't pushed that change π (just pushed it).
You should see this:
const credentialProvider= process.env.NODE_ENV == 'production' ? fromInstanceMetadata() : fromEnv();
const credentials = await credentialProvider();
I refactored the payload converter to remove some code duplication.
The test pass, so it should be fine, but could you still just check whether this works?
Yes, all still works! πͺ
π This pull request has been released in version 2.1.0-alpha.9
Thanks for your patience! I've learned a lot through the process. My next contribution will be of a higher quality:)
@mtrezza I see npm has been updated with the new release number, yet the ses provider is not present. Am I missing something? Please forgive me if I've got my wires crossed!
yet the ses provider is not present
What do you mean by "not present"?
If you look at the source code in node_modules
, it should be there. Maybe you are looking at a different version? Try to delete the node_modules
and package-lock.json
file in your app and run npm i
. Also make sure you have added the correct adapter version 2.1.0-alpha.9
in package.json
.
Hi, I've deleted the lock file, node_modules and ensured that v 2.1.0 is installed, but I still see the previous version - i.e. without the ses converter.
When I look on npm, the readme https://www.npmjs.com/package/parse-server-api-mail-adapter is also the previous version. i.e. without the ses converter.
On Thu, Sep 15, 2022 at 4:08 PM Manuel @.***> wrote:
yet the ses provider is not present
What do you mean by "not present"? If you look at the source code in node_modules, it should be there. Maybe you are looking at a different version? Try to delete the node_modules and package-lock.json file in your app and run npm i. Also make sure you have added the correct adapter version in package.json.
β Reply to this email directly, view it on GitHub https://github.com/parse-community/parse-server-api-mail-adapter/pull/65#issuecomment-1248153248, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEIB74HWEZGW6GLYVJFF6DV6MUUJANCNFSM6AAAAAAQLGZ3LM . You are receiving this because you authored the thread.Message ID: @.*** com>
Wouter Couvaras
Disclaimer: If you are not the addressee indicated in this message (or responsible for delivery of the message to such person), you may not copy or deliver this message to anyone - you should destroy this message and kindly notify the sender by reply email. Content in this message that do not relate to the official business of my employer, being Hoot Originals, shall be understood as neither given nor endorsed or authorised by it. Although all reasonable care is taken to transmit this message free of any damaging code, Hoot Originals and the sender do not make any warranties in this regard whatsoever and cannot be held liable for any loss or damages incurred by the recipient.
Β© Hoot Originals 2019. All rights reserved.
You need to look at the right version:
https://www.npmjs.com/package/parse-server-api-mail-adapter/v/2.1.0-alpha.9#providers
The item is missing in the index (I'll correct that), but if you scroll to the text it's there.
Gotcha, thanks!
On Fri, Sep 16, 2022 at 10:27 AM Manuel @.***> wrote:
You need to look at the right version:
https://www.npmjs.com/package/parse-server-api-mail-adapter/v/2.1.0-alpha.9#providers
The item is missing in the index, but if you scroll to the text it's there.
β Reply to this email directly, view it on GitHub https://github.com/parse-community/parse-server-api-mail-adapter/pull/65#issuecomment-1249075080, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEIB77LOBTOPX7PWXGNDH3V6QVO3ANCNFSM6AAAAAAQLGZ3LM . You are receiving this because you authored the thread.Message ID: @.*** com>
Wouter Couvaras
Disclaimer: If you are not the addressee indicated in this message (or responsible for delivery of the message to such person), you may not copy or deliver this message to anyone - you should destroy this message and kindly notify the sender by reply email. Content in this message that do not relate to the official business of my employer, being Hoot Originals, shall be understood as neither given nor endorsed or authorised by it. Although all reasonable care is taken to transmit this message free of any damaging code, Hoot Originals and the sender do not make any warranties in this regard whatsoever and cannot be held liable for any loss or damages incurred by the recipient.
Β© Hoot Originals 2019. All rights reserved.
π This pull request has been released in version 2.1.0-beta.2
π This pull request has been released in version 2.2.0
New Pull Request Checklist
Issue Description
Add support for an AWS SES Adapter
Related issue: #64
Approach
Very simply, I've just added a transformer to match the required SES payload for the v3 SDK. I've also updated two parameter names in the documentation as they did not match the original Parse payload I was receiving. I've also added an example of how to use it in the docs.
This is my first ever open source PR, so happy for any guidance :)
TODOs before merging