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

Enhance alias logs function #89 #62 #90

Merged
merged 7 commits into from
Mar 7, 2018

Conversation

rklhui
Copy link

@rklhui rklhui commented Jan 5, 2018

  • alias logs will now pull the latest version logs from “Lambda”
    service directly if --version flag is not specificed.
  • Added --version flag to alias logs, so that user can view logs of a
    specific version of function.

Closes #89
Closes #62

* alias logs will now pull the latest version logs from “Lambda”
service directly if --version flag is not specificed.
* Added --version flag to alias logs, so that user can view logs of a
specific version of function.
Roy Hui added 2 commits January 5, 2018 15:46
return rejected promise instead of just throw error
Copy link
Member

@HyperBrain HyperBrain left a comment

Choose a reason for hiding this comment

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

Hey @rklhui . Thanks for the PR and your work 👍 .

I have marked some changes that need to be done. Additionally, after that, the unit tests have to be adapted and extended to test for the new option and error message.
Please let me know, if you need any help there.

lib/logs.js Outdated
@@ -124,6 +125,10 @@ module.exports = {
},

functionLogsShowLogs(logStreamNames) {
if (logStreamNames.length === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Use lodash's _.isEmpty(logStreamNames) here. This will also work for cases where logStreamNames might be nil.

lib/logs.js Outdated
let aliasGetAliasFunctionVersion
// Check if --version is specified
if (this._options.version) {
aliasGetAliasFunctionVersion = BbPromise.resolve(this._options.version)
Copy link
Member

Choose a reason for hiding this comment

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

There are some missing semicolons. Please check the Travis build (https://travis-ci.org/HyperBrain/serverless-aws-alias/builds/325347600) for details.

this._options.stage,
this._options.region)
.then(result => {
return result.FunctionVersion
Copy link
Member

Choose a reason for hiding this comment

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

You can use the short form here:

.then(result => _.get(result, 'FunctionVersion', null));

With lodash that will also cover all cases where result might be invalid and returns a reasonable default.
Of course the result should then be checked at the caller (above).

Roy Hui added 3 commits January 5, 2018 17:49
added missing semicolon
use lodash to avoid errors
updated test cases for logs to match the enhancement
@rklhui
Copy link
Author

rklhui commented Jan 5, 2018

Thanks @HyperBrain, I have problem passing the test case for #updateAlias(), as I ran the test locally and seems to have no problem. Please have a look.

@HyperBrain
Copy link
Member

@rklhui I will have a look

@HyperBrain HyperBrain merged commit 8a1bf52 into serverless-heaven:master Mar 7, 2018
defnotrobbie pushed a commit to 1stdibs/serverless-aws-alias that referenced this pull request Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants