-
Notifications
You must be signed in to change notification settings - Fork 71
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
Add lambda support #41
Conversation
@@ -88,9 +88,11 @@ case class `AWS::IAM::Role`( | |||
Path: Option[Token[String]] = None, | |||
Policies: Option[Seq[Policy]] = None, | |||
override val Condition: Option[ConditionRef] = None | |||
) extends Resource[`AWS::IAM::Role`]{ | |||
) extends Resource[`AWS::IAM::Role`] with HasArn { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A left over from your previous PR?
Thank you very much for this, and all your other contributions! Overall the code looks very good. I've made a few minor comments that we'd love to hear your thoughts on. Eventually, it seems we would like to address the fact that the S3 and ZipFile options in Code are mutually exclusive, and that ZipFile is only an option with the nodejs RunTime. We may also want to start thinking about modeling ARNs in some way. Thanks again! |
Yeah, I was going back and forth about the s3 and zipFile. It depends on how far from the cloudformation 1-1 primitives should this library go? |
The primitives can be 1:1, but there are a few ways we make them exclusionary either at compile or run time. Thanks for making the other changes. |
BatchSize: Option[Token[Int]], | ||
Enabled: Option[Token[Boolean]], | ||
EventSourceArn: Token[String], | ||
FunctionName: Token[String], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This FunctionName should probably be Token[ResourceRef[
AWS::Lambda::Function]]
too.
@tylersouthwick can you change that second FunctionName type to |
both FunctionName paramters are Token[ResourceRef[ |
Thanks @tylersouthwick . Perhaps @tj-corrigan can merge this in? |
import com.monsanto.arch.cloudformation.model._ | ||
import spray.json.{JsString, JsValue, JsonFormat, DefaultJsonProtocol} | ||
|
||
class Runtime(val runtime: String) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not used Lambda for almost 1 year.
However, if we know the universe of runtime
values, then I believe using an algrebraic data type should be considered.
sealed trait Runtime
case object Java8 extends Runtime
case object ...
thanks for the PR, I've pushed out a new version (3.0.5) |
No description provided.