Add azurerm_data_factory_trigger_tumbling_window resource #9127
Conversation
ghost
added
the
dnlbunting
changed the title
dnlbunting
changed the title
|
Hey @tombuildsstuff @mbfrahry would it be possible to get a review on this, still keen to get this resources merged, thanks! |
There was a problem hiding this comment.
Thanks for the PR @dnlbunting - sorry its taken so long to review. I've given this a look over and it's off to a great start. Iv'e left some comments inline to address and it should be good to merge after they are resolved.
|
|
||
| Manages a Tumbling Window Trigger inside an Azure Data Factory. | ||
|
|
||
| Datafactory triggers are created in the "Stopped" state and must be manually enabled to start triggering. The API prevents updates to triggers in the "Started" state, so they must be manually paused before running terraform apply. |
There was a problem hiding this comment.
could we pause them, update, and restart if needed?
| resource_group_name = "example" | ||
|
|
||
| pipeline_parameters = { | ||
| example = "@{formatDateTime(trigger().outputs.windowStartTime,'yyyy-MM-dd')}" |
There was a problem hiding this comment.
could we pass this in formatted instead of this rather unclear series of functions
|
|
||
| * `data_factory_name` - (Required) The name of the Azure Data Factory to associate the Trigger with. Changing this forces a new Tumbling Window Trigger to be created. | ||
|
|
||
| * `pipeline_name` - (Required) The name of the pipeline to be triggered, there is a one to one relationship between pipelines and Tumbling Window Triggers. |
There was a problem hiding this comment.
would this make more sense as
| * `pipeline_name` - (Required) The name of the pipeline to be triggered, there is a one to one relationship between pipelines and Tumbling Window Triggers. | |
| * `triggered_pipeline_name` - (Required) The name of the pipeline to be triggered, there is a one to one relationship between pipelines and Tumbling Window Triggers. |
| }) | ||
| } | ||
|
|
||
| func TestAccAzureRMDataFactoryTriggerTumblingWindow_complete(t *testing.T) { |
There was a problem hiding this comment.
as this updates it should be
| func TestAccAzureRMDataFactoryTriggerTumblingWindow_complete(t *testing.T) { | |
| func TestAccAzureRMDataFactoryTriggerTumblingWindow_update(t *testing.T) { |
|
hey @dnlbunting - as its been about a month with no response and there are conflicts now i'm going to close this PR. but once you have time and fix it up please to open a new PR and we can get this merged! |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Closes #9126
Couple of things to call out for the reviewer:
The triggers API manages trigger start/stopping separately from create/destroy, this resource is dealing touching start/stop, but the API won't allow updates to a running trigger, which can cause apply to fail with an error telling the user to pause the trigger and try again. I'm open to suggestion about the best way to handle this behaviour.
There's validation in the trigger_dependency block that I couldn't figure out how to handle at plan time, namely if
triggeris not setoffsetmust be negativeI added additional TestChecks that that verify that the trigger can successful be started, which should catch the class of bugs like Field pipelineReference missing in Data Factory triggers #6869, and stopped again with no diff because the going though a start/stop cycle changes the result of API get slightly, stuff like "" -> nil
I'm pretty new to Go so if anything looks awkward to not idiomatic let me know :)