garnaat / kappa

What precedes Lambda
http://kappa.readthedocs.org/en/develop/
Apache License 2.0
902 stars 88 forks source link

LambdaFunctionArn shouldn't be suffixed with the environment in the S3 event source #80

Closed iserko closed 8 years ago

iserko commented 8 years ago

When configuring the S3 bucket event the function add builds the LambdaFunctionConfigurations which specifies the LambdaFunctionArn. The ARN should be the functions exact ARN and not suffixed by the environment ex.arn:aws:lambda:us-west-1:1234567890123:function:bigquery_commit:main-env.

It makes the Lambda UI and the S3 event sources UI confused as they don't recognize that the specific Lambda function even exists.

The reason why I made this change is because I had problems applying the bucket event cross-account and could not do this without this change.

Update: ok I see that anything that comes after the function ARN is a version and/or alias http://docs.aws.amazon.com/lambda/latest/dg/versioning-aliases-permissions.html ... just means that the UI can't handle these

Comments about this?

Miserlou commented 8 years ago

+1 +1 +1!

Please merge and publish ASAP!

Related to

https://github.com/garnaat/kappa/issues/81 and https://github.com/Miserlou/Zappa/issues/225

iserko commented 8 years ago

@Miserlou this only tackled the S3 event source and not the SNS event source as specified in Miserlou/Zappa#225 (amended the title because of it) ... we'd need to extend this into all the event sources to fix that specific issue

josegonzalez commented 8 years ago

@iserko can you do that?

iserko commented 8 years ago

@josegonzalez do you have any comments on how versioning is done at the moment?

Miserlou commented 8 years ago

From the code, it looks like S3 is the only odd one out, all the other event sources behave like the PR already.

iserko commented 8 years ago

@Miserlou hah you're right 😄

@josegonzalez this should be fine to merge as it basically brings it in sync with the rest of the event sources

iserko commented 8 years ago

This change was introduced by https://github.com/garnaat/kappa/pull/75 @msessa-cotd can you comment on why you added the function suffix in that PR? Your PR says

Also fixes a bug on the S3 datasource where the lambda function ARN does not include the qualifier.

In what way was it a bug?

Miserlou commented 8 years ago

Bump! Any reason this can't be merged?

iserko commented 8 years ago

@Miserlou I'm happy for this to be merged ... up to @josegonzalez

Miserlou commented 8 years ago

Can we get a 6.0.1? :-) @josegonzalez