Custom ScheduleEvent rule description#257
Conversation
Update example in sample event_sources.json
DeviaVir
left a comment
There was a problem hiding this comment.
good addition, but would like to see it slightly changed
lib/event_sources.json.example
Outdated
| "ScheduleState": "ENABLED", | ||
| "ScheduleExpression": "rate(1 hour)" | ||
| "ScheduleExpression": "rate(1 hour)", | ||
| "ScheduleDescription": "Run node-lambda-test-function once per hour" |
There was a problem hiding this comment.
if the default ${params.ScheduleName} - ${params.ScheduleExpression} is "enough" in 99% of the cases, it doesn't make sense to me to set this here and overwrite it for all functions.
Would it make more sense to not have this value here, but simply document it?
There was a problem hiding this comment.
This makes sense and I totally agree. Since there's no section dedicated to ScheduleEvents in the README, I think adding one to document some of the nuances with this feature is warranted. Anyway, if you agree I'd be more than happy to write up the documentation on it.
There was a problem hiding this comment.
@EnriqueCanals that would be great! Thanks for taking the time to make this project better!
lib/schedule_events.js
Outdated
| ScheduleEvents.prototype = { | ||
| _ruleDescription: (params) => { | ||
| return `${params.ScheduleName} - ${params.ScheduleExpression}`; | ||
| if (params.ScheduleDescription != null) { |
There was a problem hiding this comment.
this would also mean that this would have to check if ScheduleDescription is in params, and not null
Remove optional ScheduleEvents param from example event_sources.json
| #### Optional Parameter | ||
| When using the eventSourceFile flag (-S or --eventSourceFile) to set a ScheduleEvent trigger, you can pass an optional _ScheduleDescription_ key into the ScheduleEvent object with a custom description for the CloudWatch event rule you are defining. By default, node-lambda generates a _ScheduleDescription_ for you based on the ScheduleName and ScheduleExpression of the rule. | ||
|
|
||
| #### Note on ScheduleState for ScheduleEvents |
There was a problem hiding this comment.
@DeviaVir I added this subsection to the ScheduleEvents section because I feel this behavior deserves to be noted as it may be confusing to someone trying to deploy a Lambda with a disabled trigger. I wanted to bring it to your attention because I'm not certain if this is the intended behavior, but I can confirm that this is, in fact, how it currently works.
There was a problem hiding this comment.
Maybe not exactly how we like it, but long as it's been documented I think we're in a better state than before already. Thanks for that!
This PR adds the ability to provide a custom description for a ScheduleEvent rule in the eventSourceFile. Although I find the default description of
params.ScheduleName - params.ScheduleExpressionto be sufficient in most cases, I think it would be useful to also have the ability to provide a custom description for these rules.