Invoke Function Without Parameters #604

akkaash · 2018-08-03T23:42:09Z

*Issue #556 *

Invoke Function Without Parameters

Adds support to sam local invoke to accept no event argument.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

jfuss

This looks good overall. Just some small feedback

jfuss · 2018-08-08T14:14:26Z

samcli/commands/local/invoke/cli.py

+            event_data = "{}"
+        elif no_args and event != STDIN_FILE_NAME:
+            # Do not know what the user wants. no_args and event both passed in.
+            raise UserException("no_args and event both given. Do not know what to do")


Can you update this to some thing of the effect: "no_args and event cannot be used together. Please provide only one."

jfuss · 2018-08-08T14:28:49Z

samcli/commands/local/invoke/cli.py

              help="JSON file containing event data passed to the Lambda function during invoke. If this option "
                   "is not specified, we will default to reading JSON from stdin")
+@click.option("--no-args", is_flag=True, default=False, help="Invoke Function without parameters")


Thinking about this more, I don't think --no-args describes what this does. This is really giving a default value to the event. Maybe something like --no-event (so it matches with the rest of the command) and then update the help text to be Invoke the Function with an empty event dictionary?

@sooddhruv Do you have any thoughts here?

How about not requiring the --no-args/--no-event option? sam local invoke "FunctionName" could just invoke the function with a default payload.

@sooddhruv sam local invoke "FunctionName" currently allows the user to use STDIN to provide the event payload. This would cause a conflict with current behavior. What do you think?
@jfuss I agree. I like --no-event better for the option name.

@akkaash That makes sense. I can see an argument for changing this behavior but something maybe we shouldn't do now. Let's just leave this as --no-event and then we can see if there is a path/reason to change this behavior to what @sooddhruv suggested.

jfuss · 2018-08-08T14:30:58Z

tests/integration/local/invoke/test_integrations_cli.py

@@ -94,3 +94,14 @@ def test_invoke_when_function_writes_stderr(self):
        process_stderr = b"".join(process.stderr.readlines()).strip()

        self.assertIn("Docker Lambda is writing to stderr", process_stderr.decode('utf-8'))
+
+    def test_invoke_raises_exception_with_noargs_and_event(self):


Can you add a small integ test for providing --no-agrs and making sure the function invoked and returns as expected?

jfuss · 2018-08-10T21:22:01Z

@akkaash Can you rebase and handle the conflicts? The PR with the addition of --region is conflicting here. Should be a straight forward thing to resolve.

akkaash · 2018-08-13T22:02:56Z

@jfuss I've updated the pr. Multiple merge conflicts had to be fixed.

jfuss

@akkaash Two small comments on the changes and then I think this will be ready to merge. 🎉

jfuss · 2018-08-14T13:16:09Z

tests/unit/commands/local/invoke/test_cli.py

+                       region=self.region)
+
+        msg = str(ex_ctx.exception)
+        print(msg)


Can we remove this print statement from the tests? Assuming you where using this during testing.

Woops. My bad. I've fixed that.

jfuss · 2018-08-14T13:24:39Z

samcli/commands/local/invoke/cli.py

    # Pass all inputs to setup necessary context to invoke function locally.
    # Handler exception raised by the processor for invalid args and print errors
    try:
+        if no_event and event == STDIN_FILE_NAME:


Can we move this out of the try except block like it was before. Having a tight try except is a good practice to follow.

What do you think about update the checks to something like this? I think it is a little easier to reason and read (less mental checks). This is optional as I am ok with what you have as well. Just trying to see if we could simplify these checks in some manner.

if no_event and event != STDIN_FILE_NAME: raise UserException() if no_event: event_data = "{}" else: event_data = _get_event(event)

I can see how this is less cognitive overload for a reader. Made the change.

jfuss

🎉 Thanks for going through the changes here!

akkaash · 2018-08-14T21:27:27Z

@jfuss is there anything else I need to do in order to merge this in? 😃

jfuss · 2018-08-15T13:19:55Z

@akkaash Nothing is needed. I had to update the branch and never got back to it after the build passed.

@mhart

* fix: Functional tests must run on localhost to work in Windows (#552) * fix: spacing typo in Log statement in start-lambda (#559) * docs: Fix syntax highlighting in README.md (#561) * docs: Change jest to mocha in Nodejs init README (#564) * docs: Fix @mhart link in README (#562) * docs(README): removed cloudtrail, added SNS to generate-event (#569) * docs: Update repo name references (#577) * feat(debugging): Fixing issues around debugging Golang functions. (#565) * fix(init): Improve current init samples around docs and fixes (#558) * docs(README): Update launch config to SAM CLI from SAM Local (#587) * docs(README): Update sample code for calling Local Lambda Invoke (#584) * refactor(init): renamed handler for camel case, moved callback call up (#586) * chore: aws-lambda-java-core 1.1.0 -> 1.2.0 for java sam init (#578) * feat(validate): Add profile and region options (#582) Currently, `sam validate` requires AWS Creds (due to the SAM Translator). This commits adds the ability to pass in the credientials through a profile that is configured through `aws configure`. * docs(README): Update README prerequisites to include awscli (#596) * fix(start-lambda): Remove Content-Type Header check (#594) * docs: Disambiguation "Amazon Kinesis" (#599) * docs: Adding instructions for how to add pyenv to your PATH for Windows (#600) * docs: Update README with small grammar fix (#601) * fix: Update link in NodeJS package.json (#603) * docs: Creating instructions for Windows users to install sam (#605) * docs: Adding a note directing Windows users to use pipenv (#606) * fix: Fix stringifying λ environment variables when using Python2 (#579) * feat(generate-event): Added support for 50+ events (#612) * feat(invoke): Add region parameter to all invoke related commands (#608) * docs: Breaking up README into separate files to make it easier to read (#607) * chore: Update JVM size params to match docker-lambda (#615) * feat(invoke): Invoke Function Without Parameters through --no-event (#604) * docs: Update advanced_usage.rst with clarification on --env-vars usage (#610) * docs: Remove an extra word in the sam packaging command (#618) * UX: Improves event names to reflect Lambda Event Sources (#619) * docs: Fix git clone typo in installation docs (#630) * docs(README): Callout go1.x runtime support (#631) * docs(installation): Update sam --version command (#634) * chore(0.6.0): SAM CLI Version bump (#635)

Invoke Function Without Parameters

4421d55

jfuss mentioned this pull request Aug 8, 2018

Issue #555: Invoke Function Without Parameters (skip STDIN) #598

Closed

jfuss requested changes Aug 8, 2018

View reviewed changes

jfuss added area/lambda-invoke stage/in-progress A fix is being worked on labels Aug 8, 2018

Akkaash Goel and others added 2 commits August 8, 2018 20:11

rename option --no-args to --no-event

361d098

Merge branch 'develop' into develop

dcad76e

akkaash and others added 2 commits August 13, 2018 14:34

Merge branch 'develop' into develop

acae4d8

fix travis ci build failure after manual resolve conflicts

b2e3dbb

jfuss requested changes Aug 14, 2018

View reviewed changes

Tidy up integ test and tighten up validation

b173615

jfuss approved these changes Aug 14, 2018

View reviewed changes

Merge branch 'develop' into develop

a650427

jfuss merged commit 799c390 into aws:develop Aug 15, 2018

jfuss added stage/accepted Accepted and will be fixed and removed stage/in-progress A fix is being worked on labels Aug 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Invoke Function Without Parameters #604

Invoke Function Without Parameters #604

akkaash commented Aug 3, 2018

jfuss left a comment

jfuss Aug 8, 2018

akkaash Aug 9, 2018

jfuss Aug 8, 2018

dhruvsood Aug 8, 2018 •

edited

Loading

akkaash Aug 9, 2018

jfuss Aug 10, 2018

jfuss Aug 8, 2018

akkaash Aug 9, 2018

jfuss commented Aug 10, 2018

akkaash commented Aug 13, 2018

jfuss left a comment

jfuss Aug 14, 2018

akkaash Aug 14, 2018

jfuss Aug 14, 2018

akkaash Aug 14, 2018

jfuss left a comment

akkaash commented Aug 14, 2018

jfuss commented Aug 15, 2018

Invoke Function Without Parameters #604

Invoke Function Without Parameters #604

Conversation

akkaash commented Aug 3, 2018

jfuss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhruvsood Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfuss commented Aug 10, 2018

akkaash commented Aug 13, 2018

jfuss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfuss left a comment

Choose a reason for hiding this comment

akkaash commented Aug 14, 2018

jfuss commented Aug 15, 2018

dhruvsood Aug 8, 2018 •

edited

Loading