New Resource: azurerm_data_factory_trigger_schedule
#4793
Conversation
mbfrahry
commented
Nov 1, 2019
mbfrahry
commented
ghost
added
| "github.com/Azure/go-autorest/autorest/date" | ||
| "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" | ||
| "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" | ||
| "log" | ||
| "regexp" | ||
| "time" | ||
|
|
||
| "github.com/Azure/azure-sdk-for-go/services/datafactory/mgmt/2018-06-01/datafactory" | ||
| "github.com/hashicorp/terraform-plugin-sdk/helper/schema" |
There was a problem hiding this comment.
could we run goimports on this file?
| }, | ||
|
|
||
| // There's a bug in the Azure API where this is returned in lower-case | ||
| // BUG: https://github.com/Azure/azure-rest-api-specs/issues/5788 |
There was a problem hiding this comment.
I suggest reopening this on the sdk as they seem to actually act on issues there... sometimes 😪
| Optional: true, | ||
| Default: "UTC", | ||
| ValidateFunc: validate.NoEmptyStrings, | ||
| }, |
There was a problem hiding this comment.
We have a few different timezone validation functions, perhaps one would work here?
There was a problem hiding this comment.
I'm hesitant to add any validation here and might actually just remove this value. It doesn't seem to do anything and the UI just defaults the time to UTC anyway
There was a problem hiding this comment.
If you pick PST the API still takes the value as UTC?
Then i would definitely remove it, add a note that the time is always UC, and open a bug on the SDK with a link in the code explaining why UTC has been ignored.
There was a problem hiding this comment.
The Data Factory API attributes tend to accept any value without validation. You can submit anything to timezone and it'll be accepted whether it's valid or not but Pacific Standard Time didn't change the format of the time at all so I've submitted an issue on the sdk as a start. Azure/azure-sdk-for-go#6244
mbfrahry
changed the title
azurerm_data_factory_trigger_schedule azurerm_data_factory_trigger_schedule
|
Review addressed but I need to add one more piece before it's ready |
mbfrahry
changed the title
azurerm_data_factory_trigger_schedule azurerm_data_factory_trigger_schedule
|
This should be ready for a final review @katbyte if you think it's necessary |
| Optional: true, | ||
| Default: "UTC", | ||
| ValidateFunc: validate.NoEmptyStrings, | ||
| }, |
|
This has been released in version 1.37.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 1.37.0"
}
# ... other configuration ... |
|
@mbfrahry , I noticed you made a PR (#3244) to add support for azurerm_data_factory_pipeline. I found you added a property "activity" in your first commit (06bd322). But you deleted this property in later commit (6822cfa). After learned from the azure doc for data factory (https://docs.microsoft.com/en-us/azure/data-factory/v1/data-factory-copy-activity-tutorial-using-powershell), I think you should retain this property "activity" since I think it's an important thing for data factory pipeline to combine the InputDataset and the OutputDataset. May I know why you removed the property "activity"? If this property is necessary, can it be added back? |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
