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

(lambda): Improve FilterCriteria documentation #23214

Open
peterwoodworth opened this issue Dec 2, 2022 · 5 comments
Open

(lambda): Improve FilterCriteria documentation #23214

peterwoodworth opened this issue Dec 2, 2022 · 5 comments
Labels
@aws-cdk/aws-lambda Related to AWS Lambda documentation This is a problem with documentation. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@peterwoodworth
Copy link
Contributor

Describe the bug

The documentation here leads to a malformed template for EventSource filters

Expected Behavior

I expect the code in the docs to work, and if it doesn't, I expect to have a good enough understanding to work around the limitation in the example.

Current Behavior

The code seen here:

fn.addEventSource(new eventsources.DynamoEventSource(table, {
  startingPosition: lambda.StartingPosition.LATEST,
  filters: [{ eventName: lambda.FilterRule.isEqual('INSERT') }],
}));

will provide this output in CloudFormation

      FilterCriteria:
        Filters:
          - {}

And the example doesn't leave any good hints as to what the right thing to do is

Reproduction Steps

Synthesize the docs

Possible Solution

The correct code is this:

    fn.addEventSource(new DynamoEventSource(table, {
      startingPosition: lambda.StartingPosition.LATEST,
      filters: [FilterCriteria.filter({ eventName: lambda.FilterRule.isEqual('INSERT') })],
    }));

Additional Information/Context

No response

CDK CLI Version

latest

Framework Version

No response

Node.js Version

16

OS

mac

Language

Typescript

Language Version

No response

Other information

We need to explain how to implement different kinds of filters, rather than just providing an example

@peterwoodworth peterwoodworth added bug This issue is a bug. p1 effort/small Small work item – less than a day of effort documentation This is a problem with documentation. labels Dec 2, 2022
@peterwoodworth
Copy link
Contributor Author

@marciocadev this feature you implemented is wonderful, is there any chance you can work on explaining how it works and updating the existing example?

@marciocadev
Copy link
Contributor

Sure, I will update the docs

@marciocadev
Copy link
Contributor

marciocadev commented Dec 3, 2022

@peterwoodworth
I don't know what's going on, in the README.md of the lambda the example is like this

import * as eventsources from '@aws-cdk/aws-lambda-event-sources';
import * as dynamodb from '@aws-cdk/aws-dynamodb';

declare const fn: lambda.Function;
const table = new dynamodb.Table(this, 'Table', {
  partitionKey: {
    name: 'id',
    type: dynamodb.AttributeType.STRING,
  },
  stream: dynamodb.StreamViewType.NEW_IMAGE,
});
fn.addEventSource(new eventsources.DynamoEventSource(table, {
  startingPosition: lambda.StartingPosition.LATEST,
  filters: [lambda.FilterCriteria.filter({ eventName: lambda.FilterRule.isEqual('INSERT') })],
}));

but in the documentation the filters line is like this filters: [{ eventName: lambda.FilterRule.isEqual('INSERT') }],

@TheRealAmazonKendra can you help us to update this doc ?

@peterwoodworth
Copy link
Contributor Author

@marciocadev thanks for noticing this, it turns out we fixed this example specifically here #23085, we just haven't had a release since so I didn't notice this got fixed. My bad 😅

Regardless, are you still interested in writing up a little bit of documentation on how FilterCriteria should be used in general? I think there's some room for improvement still 🙂

@tim-finnigan
Copy link

Since the example got fixed I think we can remove the bug label? And leave this open for improving the FilterCriteria documentation.

@tim-finnigan tim-finnigan added @aws-cdk/aws-lambda Related to AWS Lambda p2 and removed bug This issue is a bug. p1 labels Jan 27, 2023
@tim-finnigan tim-finnigan changed the title (lambda): EventSource FilterCriteria documentation example doesn't synthesize filters (lambda): Improve FilterCriteria documentation Jan 27, 2023
@peterwoodworth peterwoodworth added the feature-request A feature should be added or improved. label May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda documentation This is a problem with documentation. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

3 participants