Add Twilio Runtime to create templates #6467

stefanjudis · 2019-07-29T08:30:10Z

What did you implement:

Added a new template to initialize projects using the Twilio Runtime. :)

How did you implement it:

Added a new serverless.yml, package.json, a function and an asset to the plugins create directory.

How can we verify it:

register at Twilio
export TWILIO_ACCOUNT_SID=YOUR_TWILIO_ACCOUNT_SID and TWILIO_AUTH_TOKEN=YOUR_TWILIO_AUTH_TOKEN – you'll find both in the Twilio console
git clone https://github.com/pinkerton/serverless.git
cd serverless
npm install
cd ..
./serverless/bin/serverless create --template twilio-runtime --path my-project
cd my-project
npm install
serverless deploy
serverless invoke -f hello-world
serverless info
open deployed asset from deploy logs in a browser

Screenshots

Create Twilio-runtime with local binary

Run initial deploy

Invoke deployed functions

Retrieve invormation about the new service

New asset available in Firefox

Todos:

Note: Run npm run test-ci to run all validation checks on proposed changes

Write tests and confirm existing functionality is not broken.
Validate via npm test
Write documentation
Ensure there are no lint errors.
Validate via npm run lint-updated
Note: Some reported issues can be automatically fixed by running npm run lint:fix
Ensure introduced changes match Prettier formatting.
Validate via npm run prettier-check-updated
Note: All reported issues can be automatically fixed by running npm run prettify-updated
Make sure code coverage hasn't dropped
Provide verification config / commands / resources
Enable "Allow edits from maintainers" for this PR
Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

pmuens

Hey @stefanjudis thanks for working on this! This looks really great 👍 💯

Could you add a unit test in create.test.js so that we can be sure that the correct files are generated? Furthermore you might want to do a global search for another template name (e.g. google-nodejs) to find other places we usually update when adding new templates.

Thanks again for working on this and let us know if you have any issues!

stefanjudis · 2019-07-29T12:08:29Z

Hey @pmuens, thanks so much for the swift reply... :) 👌

(sorry that I missed the tests initially - I was mainly following a previous PR)

I added a unit test, and searched the project. Unfortunately, I have to admit I'm a bit overwhelmed with the shell/docker magic in the integration tests i found. 😊

https://github.com/serverless/serverless/pull/6467/files#diff-4e5e90c6228fd48698d074241c2ba760R127

👆 Twilio functions run in Node 8.10... I'm not really familiar with docker and the overall setup. Should pick a matching image there? The tests are not really doing much in that case (if I understand that correctly) but it may be "more correct" to specify the correct node version?

Edited:
I just checked here internally and it might that we'd have to wait until the end of the week or beginning next week (in case the PR runs smoothly 🙈). The new features are not available or documented for everybody yet.

pmuens · 2019-07-30T07:24:34Z

Great! Thanks for the update @stefanjudis 👍

👆 Twilio functions run in Node 8.10... I'm not really familiar with docker and the overall setup. Should pick a matching image there? The tests are not really doing much in that case (if I understand that correctly) but it may be "more correct" to specify the correct node version?

Yes, picking the matching Node version number for the image should be sufficient. TBH we don't use such tests that often (we might even want to remove them in the future).

stefanjudis · 2019-07-30T08:26:16Z

@pmuens I took the closed Node image now... (I think that's good enough for now).

Is there anything else that I should have a look into?

As mentioned there's no rush on my end. :) Just want to be ready when we open the API. :)

pmuens

Hey @stefanjudis, thanks for updating the PR! 👍

@pmuens I took the closed Node image now... (I think that's good enough for now).

Yes, that should do the trick!

Is there anything else that I should have a look into?

We might want to add docs, but I'm not sure if this is strictly necessary for the first step, so I'd put it on hold for the time being.

As mentioned there's no rush on my end. :) Just want to be ready when we open the API. :)

Sounds good. Excited for the official announcement.

pmuens · 2019-07-31T11:39:02Z

lib/plugins/create/templates/twilio-runtime/handler.js

@@ -0,0 +1,11 @@
+const logo = require('asciiart-logo');


Where is the code for this dependency coming from? Shouldn't this be listed in the package.json dependencies section? Or is this available in the Twilio Runtime? 🤔

it's defined in the serverless.yml.

https://github.com/serverless/serverless/pull/6467/files#diff-c9fa620e15d789a64e35aeb136e52e5bR16-R17

I didn't want to mix this with the dependencies defined in the package.json. Several other parts of our tooling define the function dependencies in a separated config. For the case of serverless framework I thought this makes sense to have all the config in the serverless.yml. WDYT?

Interesting. Haven't seen that 🤔

Thanks for clarifying 👍

The other provider plugins are implemented in a way that one has to explicitly define them in package.json (or whatever runtime / package manager you're using).

I know that this is not well documented but we suggest to put everything proprietary (which is not part of the original serverless.yml spec) under the custom property. This way the Framework can be extended alter on without causing name collision.

TBH I'm fine with either way (whatever is best for Twilio), but I'd suggest to move this property to custom:

custom: dependencies: asciiart-logo: '*'

Thanks for having another look. @pmuens

Your points are very interesting and bring up an additional question on my end. I had a look at the initial AWS serverless.yml and it turns out my proposal is different not only in the dependency part.

I defined the following different properties:

provider.environment - which has a completely different meaning then in the AWS config (the reason for that is that in the twilio runtime environment is similar to what a stage in AWS is – I decided to follow naming conventions in our docs)

provider.config - needed configuration token (I more or less copied that approach from the cloudflare serverless plugin implementation 🙈)

provider.environmentVars - the environment variables available in the service

provider.dependencies - needed npm deps which will be provided

I feel like it will be tough to match the AWS keys/properties because there are some major differences and not everything is mappable.

About the custom approach – I don't have strong feeling about this but I'm not sure if that's not more confusing to users to have important things in a not related custom property. Dependency definitions, auth credentials and environment configuration belongs on the provider level for me.

I also looked at the other providers and it feels like there a many differences already. :)

Long story short – it feels like I won't be able to match every needed config in Twilio Runtime to AWS properties and am not sure about the custom property. 🙈 I'm more than happy to rename keys but for now would prefer to have everything in the provider (if it's really cool for you?).

Friendly ping @pmuens. 😊

As far as I see this is the missing part to move forward with this PR. :)

Hey @stefanjudis,
sorry for the slow response on my end. This somehow slipped through the cracks, so thanks for pinging me 👍

I agree after reading your comment here. Since this is a provider plugin it's fine to have the important config parts at the provider level. I furthermore agree that putting things into different places will just cause confusion, so the choices you made here are absolutely right 👌

stefanjudis · 2019-07-31T12:20:56Z

We might want to add docs, but I'm not sure if this is strictly necessary for the first step, so I'd put it on hold for the time being.

I'm assuming you're talking about this area?

I was not sure if you folks would be up for that, but I'm happy to add docs in another PR. 🎉

pmuens · 2019-08-01T07:52:34Z

I'm assuming you're talking about this area?

Yes, exactly 👍

I was not sure if you folks would be up for that, but I'm happy to add docs in another PR. 🎉

Having such docs would be super valuable!

stefanjudis · 2019-08-01T09:13:16Z

Nice! 🎉 Let's tackle docs then right after we have this here shipped. :) 🎉

pmuens · 2019-08-02T08:31:46Z

Nice! 🎉 Let's tackle docs then right after we have this here shipped. :) 🎉

Sounds good! 🎉

pmuens

This LGTM

Thanks again for taking the time to work on this @stefanjudis 👍

Just wanted to mention that we have an examples GitHub project where we share some common example projects with the community. It might be a good idea to add some basic example over there as well since it's quite popular among newcomers.

Eagerly waiting for the docs. Let us know if you need any help from us!

pmuens · 2019-08-07T10:56:55Z

Just another quick question after working with the Twilio Runtime. Does this runtime focus on Node.js? Will it be possible to use other languages as well?

I'm asking since our naming schema for Serverless templates usually is {provider}-{runtime} / {provider}-{runtime}-{build-tool}.

Given that it might be worthwhile to rename the template to twilio-nodejs. This way it's more future proof and clear that one has to work with Node.js.

stefanjudis · 2019-08-07T11:10:06Z

Given that it might be worthwhile to rename the template to twilio-nodejs

We'll only Node and this is a very valid point. Will push an update in the next 2h and give you another ping. :)

Thanks!!! 🎉

stefanjudis · 2019-08-07T11:42:32Z

@pmuens Alright. I just gave it another local try after renaming and everything still works. :)

I think we're ready to go. 🎉🎉🎉

I'll add an example to the examples repo after we merged this PR and after the example, I'll have a look at the docs section.

pmuens

@pmuens Alright. I just gave it another local try after renaming and everything still works. :)
I think we're ready to go. 🎉🎉🎉

Great! Thanks for updating this @stefanjudis 👍

I'll add an example to the examples repo after we merged this PR and after the example, I'll have a look at the docs section.

That sounds great! Thanks again for working so hard on this! We really appreciate that! 💯 🥇

pmuens added cat/template feature labels Jul 29, 2019

pmuens suggested changes Jul 29, 2019

View reviewed changes

pmuens added this to the 1.50.0 milestone Jul 29, 2019

pmuens self-assigned this Jul 29, 2019

Add Twilio Runtime to create template

8da1e2b

pmuens reviewed Jul 31, 2019

View reviewed changes

pmuens added pr/in-review and removed pr/in-progress labels Jul 31, 2019

pmuens previously approved these changes Aug 7, 2019

View reviewed changes

stefanjudis dismissed pmuens’s stale review via 29f86b4 August 7, 2019 11:13

Rename template 'twilio-runtime' to 'twilio-nodejs'

ba37f23

pmuens approved these changes Aug 7, 2019

View reviewed changes

pmuens merged commit 2c13156 into serverless:master Aug 7, 2019

stefanjudis mentioned this pull request Aug 7, 2019

Add Twilio Runtime node forward call example serverless/examples#431

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Twilio Runtime to create templates #6467

Add Twilio Runtime to create templates #6467

stefanjudis commented Jul 29, 2019 •

edited

pmuens left a comment

stefanjudis commented Jul 29, 2019 •

edited

pmuens commented Jul 30, 2019

stefanjudis commented Jul 30, 2019

pmuens left a comment •

edited

pmuens Jul 31, 2019 •

edited

stefanjudis Jul 31, 2019

pmuens Aug 1, 2019

stefanjudis Aug 1, 2019

stefanjudis Aug 6, 2019

pmuens Aug 7, 2019

stefanjudis commented Jul 31, 2019

pmuens commented Aug 1, 2019 •

edited

stefanjudis commented Aug 1, 2019

pmuens commented Aug 2, 2019

pmuens left a comment

pmuens commented Aug 7, 2019

stefanjudis commented Aug 7, 2019

stefanjudis commented Aug 7, 2019

pmuens left a comment

Add Twilio Runtime to create templates #6467

Add Twilio Runtime to create templates #6467

Conversation

stefanjudis commented Jul 29, 2019 • edited

What did you implement:

How did you implement it:

How can we verify it:

Screenshots

Todos:

pmuens left a comment

Choose a reason for hiding this comment

stefanjudis commented Jul 29, 2019 • edited

pmuens commented Jul 30, 2019

stefanjudis commented Jul 30, 2019

pmuens left a comment • edited

Choose a reason for hiding this comment

pmuens Jul 31, 2019 • edited

Choose a reason for hiding this comment

stefanjudis Jul 31, 2019

Choose a reason for hiding this comment

pmuens Aug 1, 2019

Choose a reason for hiding this comment

stefanjudis Aug 1, 2019

Choose a reason for hiding this comment

stefanjudis Aug 6, 2019

Choose a reason for hiding this comment

pmuens Aug 7, 2019

Choose a reason for hiding this comment

stefanjudis commented Jul 31, 2019

pmuens commented Aug 1, 2019 • edited

stefanjudis commented Aug 1, 2019

pmuens commented Aug 2, 2019

pmuens left a comment

Choose a reason for hiding this comment

pmuens commented Aug 7, 2019

stefanjudis commented Aug 7, 2019

stefanjudis commented Aug 7, 2019

pmuens left a comment

Choose a reason for hiding this comment

stefanjudis commented Jul 29, 2019 •

edited

stefanjudis commented Jul 29, 2019 •

edited

pmuens left a comment •

edited

pmuens Jul 31, 2019 •

edited

pmuens commented Aug 1, 2019 •

edited