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

PLAT-1202 - Interactive `serverless` create #6294

Merged
merged 37 commits into from Jul 10, 2019

Conversation

Projects
2 participants
@medikoo
Copy link
Member

commented Jun 24, 2019

Interactive serverless create, that allows to interactively setup a new service, or configure not yet configured feature.

Initially it proposes interactive setup of:

New service (from a chosen template).

Only if command is invoked not in a service directory.

At this point just two choices are proposed (AWS Node.js and AWS Python). This needs to be further discussed (I think current templates list is too large to propose them all)

AWS credentials

Only If command is invoked in context of AWS service and AWS credentials are not found to be setup in environment


As this work overlapped some other functionalities, this PR also includes:

  • Agnostic lib/utils/createFromTemplate.js util for creation of a new service from a template. lib/plugins/create/create.js was refactored to rely on that.
  • Agnostic lib/plugins/aws/utils/credentials.js util for handling AWS credentials. lib/plugins/aws/configCredentials/awsConfigCredentials.js was refactored to rely that, which brought in following improvments:
    • New profiles can be added via config credentials option (so far this option offered either complete overwrite or setup of new profile when non were configured)
    • Doesn't create ~/.aws/credentials at package install and in case of non AWS services. That fixes #4506
  • bin/serverless.js - Cleanup
  • lib/classes/Service.js - Improvement to options handling

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

@medikoo medikoo self-assigned this Jun 24, 2019

@medikoo medikoo added this to In progress in Serverless via automation Jun 24, 2019

@medikoo medikoo force-pushed the interactive-serverless branch 3 times, most recently from 9cec478 to c79fb83 Jun 24, 2019

@pmuens pmuens added this to the 1.47.0 milestone Jun 26, 2019

@medikoo medikoo force-pushed the interactive-serverless branch from 763c72a to 5e45102 Jun 26, 2019

medikoo added some commits Jun 21, 2019

@medikoo medikoo force-pushed the interactive-serverless branch from 5e45102 to 23b5bcf Jun 28, 2019

medikoo added some commits Jul 1, 2019

@medikoo medikoo marked this pull request as ready for review Jul 2, 2019

@medikoo medikoo added pr/in-review and removed pr/in-progress labels Jul 2, 2019

@medikoo medikoo requested a review from pmuens Jul 2, 2019

@pmuens
Copy link
Member

left a comment

Great job! I just tested the template creation and it and it works fine 👍

Also tested the credentials setup and it still works as expected.

Just added some comments. Pretty sure we can merge this quite soon!

Show resolved Hide resolved lib/plugins/aws/configCredentials/awsConfigCredentials.test.js Outdated
// rename the service if the user has provided a path via options and is creating a service
if ((boilerplatePath || serviceName) && notPlugin) {
const newServiceName = serviceName || boilerplatePath.split(path.sep).pop();
userStats.track('service_created', {

This comment has been minimized.

Copy link
@pmuens

pmuens Jul 2, 2019

Member

Oprhaned promise, but I guess this is still something we do in such situations.

This comment has been minimized.

Copy link
@medikoo

medikoo Jul 2, 2019

Author Member

Technically it's left the way it was (change is due to refactor that relocated that part in file).

Yes, we do it like that now. Idea is not bad, but the way it's implemented it's prone to issues (especially that we shortcut process with process.exit, so there's no guarantee those requests will fulfill, but it's not a concern for this PR - and we discuss it already in other channel)

@pmuens
Copy link
Member

left a comment

Just looked through this again and it's LGTM from my side :shipit:

Happy to do another review since I'm not sure what else is needed here.

Serverless automation moved this from In progress to Reviewer approved Jul 3, 2019

medikoo added some commits Jul 8, 2019

Serverless automation moved this from Reviewer approved to Needs review Jul 8, 2019

@medikoo

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

@pmuens I've updated this PR with:

  • Fix AWS setup link
  • Ensure Serverless log style (mostly by hacking inquirer's chalk)
  • Improve some formatting.

I've kept projects list to AWS Node.js and AWS Python, as I still miss necessary feedback on how to expand it further. If we're to publish this with v1.47.0 I think it needs to stay that way.

@medikoo medikoo requested a review from pmuens Jul 8, 2019

@pmuens
Copy link
Member

left a comment

This looks good so far. Just tested it again and it looks good and works fine 👍

I added a comment about the URL we're referring to. Would be nice if we could keep our own bit.ly style URL forwarder.

Show resolved Hide resolved lib/plugins/interactiveCli/setupAws.js Outdated

@medikoo medikoo requested a review from pmuens Jul 10, 2019

medikoo added some commits Jul 10, 2019

@pmuens

pmuens approved these changes Jul 10, 2019

Copy link
Member

left a comment

LGTM :shipit:

Thanks for updating @medikoo 👍

Serverless automation moved this from Needs review to Reviewer approved Jul 10, 2019

@medikoo medikoo merged commit 4564c36 into master Jul 10, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json (medikoo) No new issues
Details

Serverless automation moved this from Reviewer approved to Done Jul 10, 2019

@medikoo medikoo deleted the interactive-serverless branch Jul 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.