-
Notifications
You must be signed in to change notification settings - Fork 479
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
Added Support to Amazon.Lambda.Annotations For Sqs (SQSEvent.SQSMessage) #1203
Added Support to Amazon.Lambda.Annotations For Sqs (SQSEvent.SQSMessage) #1203
Conversation
This reverts commit b102002.
…og22030/aws-lambda-dotnet into feature/annotations-queue
This reverts commit 57b85a1.
{ Debugger.Launch(); }
Hi @hounddog22030, sorry we haven't gotten back to you sooner. We have been busy with other projects and are now refocusing on this project. I like what you are doing with syncing the CF template, although I still need dig in more. Do you have any thoughts about simplifying the parameters into the Lambda function? Or handling the return |
I can take a look @ that.
I haven't thought of that. Honestly, I've never even used that. What are your thoughts?
Please elaborate a bit more on that. Thanks. |
It has been a while since I worked on this and just took a look @ it. I thought you were asking about all of the properties on Looking @ that, I don't know how to simplify that; it's literally everything from AWS::SQS:Queue. For those, there are two examples, one for an existing queue and one to create a new queue. The existing queue example is pretty clean:
Other than that, the one that creates a new queue, almost everything is optional with defaults. For example:
Is that what you were asking about to simplify or were you asking about something entirely different? |
@normj - any thoughts on the above? |
@hounddog22030 I'm on vacation right now, but don't want you to think I'm ignoring your question. I'll catch back up on the PR when I'm back from vacation. |
np. enjoy your well deserved vacay |
@normj - any chance of looking @ this soon? thanks |
@hounddog22030 You are the best ! I really appreciate your hard work with this feature! Why this is taking so long to merge or at least provide constructive feedback? @normj. |
Description of changes:
Added support to add Event properties and values for a new attribute (
SqsMessageAttribute
) to template file in a similar manner toRestApiAttribute
andHttpApiAttribute
.Added attribute
SqsMessageAttribute
that supports all AWS::Serverless::Function Event properties of type 'SQS' and all AWS::SQS::Queue properties .Added functionality to write the
AWS::SQS::Queue
to the template using values from the attributes.Added a sample class (Messaging.cs) that demonstrates using the
SqsMessageAttribute
in its two use cases:AWS::SQS::Queue
to theEvents
property ofAWS::Serverless::Function
.AWS::SQS::Queue
and wires it up to theEvents
propertyAWS::Serverless::Function
.Added a property to
AWS::Serverless::Function
metadata in the template namedSyncedResources
that works in very similar vain toSyncedEvents
.Added tests cases following same patterns as
HttpApiAttribute
andRestApiAttribute
in classCloudFormationJsonWriterTests
.Added test cases to check the bounds of all the properties in
SqsMessageAttribute
.This is a large change and there are specific places where I had a hard time following the patterns of
RestApiAttribute
andHttpApiAttribute
. Otherwise, I tried to followed those patterns very closely. Those three places are:[assembly: InternalsVisibleTo]
inSqsMessageAttribute.cs
- there is a TODO that documents this._processedResources
inCloudFormationJsonWriter.cs
- this is probably not a big deal, but let me know.* I added a(I figured out how to get around this, it has been removed).<VersionPrefix>
for our build process in order to be able to push to a nuget repo with a unique version number.I made several attempts to go thru and remove any formatting issues, but a few small ones remain. So, hopefully the review will be straightforward.
I have updated the readme.md with description of the attribute and provided two examples. All public properties are documented in
ISqsMessage
with<summary>
.This was originally created off of the
annotations
branch, butmaster
was merged in.The commit comments will probably be nonsensical as I do a lot of iterating (over 100 commits).
Thanks for reviewing.
This code will produce a template similar to:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.