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

Throw an error if plugin is executed outside of a serverless directory #5636

Merged
merged 7 commits into from Jan 31, 2019

Conversation

Projects
4 participants
@shanehandley
Copy link
Contributor

shanehandley commented Dec 30, 2018

What did you implement:

This is a revisit of a previous developer experience bug where executing a serverless command in a path with no serverless.yml would fail silently. I noticed that this issue has returned in the latest release.

The tests appeared to be passing because the method to validate a plugins config dependency was working, but the rejection was not being caught during execution, possible because of the asynchronous nature of loading the config file in getServerlessConfigFile

How did you implement it:

getServerlessConfigFile was being called twice within the pluginManager, so serverless will now ask the PluginManager to load the config file earlier and then store it to lookup during plugin execution, rather than try to load in it's methods as it was previously.

How can we verify it:

Currently, running sls info provides no feedback when running in a directory without a serverless.yml will result in a silent failure.

Checking out this commit and try running ./bin/serverless info in a directory with no .serverless.yml will result in an error indicating that it must be executed in a serverless directory.

No documentation has been added as it's N/A.

Todos:

  • Write tests
  • Fix linting errors
  • 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

shanehandley added some commits Dec 30, 2018

@pmuens pmuens added the pr/in-review label Jan 15, 2019

@eahefnawy eahefnawy self-assigned this Jan 17, 2019

@eahefnawy eahefnawy self-requested a review Jan 17, 2019

@eahefnawy eahefnawy added this to In progress in Serverless via automation Jan 17, 2019

@eahefnawy

This comment has been minimized.

Copy link
Member

eahefnawy commented Jan 17, 2019

thanks @shanehandley ... and really sorry for the late reply here! 😊 ... I'm currently reviewing this.

@eahefnawy eahefnawy moved this from In progress to Needs review in Serverless Jan 17, 2019

Serverless automation moved this from Needs review to Reviewer approved Jan 31, 2019

@eahefnawy
Copy link
Member

eahefnawy left a comment

Thanks @shanehandley ... just tested this out and it works as expected!

LG2M

@eahefnawy eahefnawy added this to the 1.36.4 milestone Jan 31, 2019

@eahefnawy eahefnawy merged commit 96e8ea6 into serverless:master Jan 31, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.009%) to 90.79%
Details

Serverless automation moved this from Reviewer approved to Done Jan 31, 2019

@eahefnawy eahefnawy referenced this pull request Feb 5, 2019

Merged

Fix race condition when loading config file #5793

0 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment