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

UX: Improves event names to reflect Lambda Event Sources #619

Merged
merged 7 commits into from
Aug 22, 2018

Conversation

heitorlessa
Copy link
Contributor

@heitorlessa heitorlessa commented Aug 17, 2018

Issue #, if available:

Description of changes:

This PR changes the local event generation for SQS, CodeCommit, Kinesis, Cloudwatch and SNS to reflect what the event and the event source means to Lambda:

Before After
sam local generate-event sqs sqs sam local generate-event sqs receive-message
sam local generate-event sns sns sam local generate-event sns notifications
sam local generate-event kinesis kinesis sam local generate-event kinsis get-records
sam local generate-event codecommit codecommit sam local generate-event codecommit repository
sam local generate-event logs cloudwatch-logs sam local generate-event cloudwatch logs
sam local generate-event events cloudwatch sam local generate-event cloudwatch scheduled-event

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

@jfuss
Copy link
Contributor

jfuss commented Aug 17, 2018

Can we expand the scope here while we are touching this. I just went through all of the events again. Here are some things that could be confusing to customers.

Other duplicates:

  • codecommit
  • sns
  • kinesis

We have two CloudWatch related events (logs and events). Shouldn't this follow the service to type event model we have? So sam local generate-event event events and sam local generate-event logswould be combined intosam local generate-event cloudwatchwith two sub commands ofscheduled-eventandlogs`?

@sooddhruv What are your thoughts? We should find better names to the duplicate like @heitorlessa has done with SQS.

@heitorlessa
Copy link
Contributor Author

heitorlessa commented Aug 18, 2018 via email

@dhruvsood
Copy link
Contributor

Thanks for fixing this, @heitorlessa. I agree with @jfuss. Please increase the scope of this change to cover other services with duplicate commands.

@heitorlessa heitorlessa changed the title fix: Replaces sqs event name to explicit receive-message UX: Improves event names to reflect Lambda Event Sources Aug 21, 2018
@heitorlessa
Copy link
Contributor Author

@sooddhruv and @jfuss - Done!

PR description updated and table with changes to reflect the new scope

@dhruvsood
Copy link
Contributor

👍

Copy link
Contributor

@dhruvsood dhruvsood left a comment

Choose a reason for hiding this comment

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

👍 Thanks, @heitorlessa!

@jfuss
Copy link
Contributor

jfuss commented Aug 21, 2018

@heitorlessa Should kinesis get-records be kinesis get-record and sns notifications be sns notification (singular). There are only one record in each list. This is really nit picky but want to make sure we communicate the right thing here.

@heitorlessa
Copy link
Contributor Author

heitorlessa commented Aug 22, 2018 via email

@heitorlessa
Copy link
Contributor Author

@jfuss pushed SNS changes and had a second link at Kinesis and by making this change (2 records) it'd have to increase the scope of parameters to change data + sequence + partition which it'd make it overly complex.

Given that the API Call is GetRecords and a Kinesis Data Stream can have a single or multiple records I'd vote to keep as-is otherwise we could likely diverge from its essence - Happy to hear otherwise

@jfuss
Copy link
Contributor

jfuss commented Aug 22, 2018

@heitorlessa Sounds reasonable. Thanks for updating.

@jfuss jfuss merged commit 6e3740a into aws:develop Aug 22, 2018
jfuss added a commit that referenced this pull request Aug 28, 2018
* 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)
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.

3 participants