search

xpring-eng / payid-lambda

AWS Lambda setup for a PayID server
4 stars 3 forks source link

Question about proposing verified PayID support #6

Closed rswarthout closed 4 years ago

rswarthout commented 4 years ago

I have forked and have a change I would like to submit a pull request for that adds verified PayID support.

The issue I am running into with CloudFormation is that it will not allow the stack to create due to the 4096 character limit for ZipFiles that are embedded into a CloudFormation template. The only way I see around this is to keep the function in a separate file within the repo, minify it, and build it into the YAML file.

Is there a better way to deal with this or to any other concerns with this approach?

theotherian commented 4 years ago

Ouch; yeah, I ran into that issue recently when trying to make a change and ended up backing it out. You can make it an external file, but that also means it has to be uploaded to S3 first and referenced that way:

https://stackoverflow.com/questions/54876833/upload-local-file-on-parameter-zipfile-awslambdafunction

To my knowledge, there's not a way of uploading it directly from a path relative to the CloudFormation template. Uploading it to S3 isn't a huge feat; it's mostly just annoying. I have to upload the template to S3 on every release anyway because the button on the repo requires it.

I'm not super into the idea of minifying it to dodge the 4k limit because it has the unfortunate side effect of making the code an unreadable mess, and part of the motivation with the Lambda was having the code in a quasi-IDE that people could "touch" as they're getting more familiar with PayID.

Would it work if I create a branch that refactors the CloudFormation template into using an S3 bucket, and have you try to merge your fork into that branch instead of master?

rswarthout commented 4 years ago

Ok, makes sense to me. Yes, feel free to refactor and then ping me and I can work my changes into the needed places.

theotherian commented 4 years ago

Ok cool. I created a branch here: https://github.com/xpring-eng/payid-lambda/tree/externalize-function

The lambda function was refactored into a file called lambda-function.js. Let me know if this works as a jumping off point to get your changes merged in. I haven't started working on the actual task of using the S3 file yet, but I'm hoping just having the external file that has to be uploaded defined gives you what you need for now.

I probably won't be able to look at doing the refactoring until next week.

rswarthout commented 4 years ago

Yes, this works for me. Thanks.

rswarthout commented 4 years ago

Closing this as the PR has been merged.