New Resource: azurerm_data_factory_dataset_cosmosdb_mongoapi #28185

gerrytan · 2024-12-05T01:35:09Z

Community Note

Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

Adds a new resource for Data Factory CosmosDB MongoDB dataset. The resource is modelled similar to data_factory_dataset_cosmosdb_sqlapi

Resource relationship:

azurerm_data_factory --> azurerm_data_factory_linked_service_cosmosdb_mongoapi --> azurerm_data_factory_dataset_cosmosdb_mongoapi

Resource screenshot in portal:

PR Checklist

I have followed the guidelines in our Contributing Documentation.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Testing

My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

acctest log: acctests_TestAccDataFactoryDatasetCosmosDbMongoDb_7358021176_20241205-1121+1100.log

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

azurerm_data_factory_dataset_cosmosdb_mongoapi - new resource [Cosmos DB MongoDB-API Data Factory dataset #27106]

This is a (please select all that apply):

Bug Fix
New Feature (ie adding a service, resource, or data source)
Enhancement
Breaking Change

Related Issue(s)

Closes #27106

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

magodo

Thanks for this PR!
I've taken a look through and left some comments inline, but this is mostly looking good to me 👍

internal/services/datafactory/data_factory_dataset_cosmosdb_mongoapi_resource.go

internal/services/datafactory/data_factory.go

internal/services/datafactory/data_factory_dataset_cosmosdb_mongoapi_resource_test.go

- Reorder schema properties - Change AdditionalProperties model type to map[string]string - Change Parameters to map[string]string - Use existing utils.FlattenStringSlice instead of creating a new function - acctest: update: add 1 more step to update back to basic - terrafmt fix for acctest and docs

…source

magodo · 2024-12-09T05:17:46Z

LGTM! Thank you @gerrytan!

gerrytan · 2024-12-09T05:39:51Z

Acctest passes as of commit f65b76f : acctests_TestAccDataFactoryDatasetCosmosDbMongoDb_20241209-1628+1100.log

magodo · 2024-12-09T06:29:58Z

jackofallops

Hi @gerrytan - Few questions and observations below, the key one is the choice of SDK for this new resource. We should prefer go-azure-sdk over kermit unless there's a compelling technical reason as we are attempting to discontinue the use of kermit entirely.
If you can take a look I'll continue review. Thanks!

jackofallops · 2024-12-09T06:41:31Z

website/docs/r/data_factory_dataset_cosmosdb_mongoapi.html.markdown

+
+* `folder` - (Optional) The folder that this Dataset is in. If not specified, the Dataset will appear at the root level.
+
+* `parameters` - (Optional) A map of parameters to associate with the Data Factory Dataset.


Are these exclusively strings? Are there no defined keys for these parameters? If only string values are supported here for technical reasons, it should be called out as a note?

Have added additional info that only string -> string is supported. I decided not to support other parameter type to maintain consistency with the other _dataset_ resources in the folder.

d95a18f

jackofallops · 2024-12-09T07:39:52Z

internal/services/datafactory/data_factory_dataset_cosmosdb_mongoapi_resource.go

@@ -0,0 +1,339 @@
+package datafactory


This filename seems so be mismatched from the resource name? Should it be data_factory_dataset_cosmosdb_mongodb_resource.go, or should the resource model be named DataFactoryDatasetCosmosDbMongoAPIResource instead?

Good catch, have fixed this into _mondodbapi_resource and corresponding go model type.

d95a18f

jackofallops · 2024-12-09T08:00:26Z

internal/services/datafactory/data_factory_dataset_cosmosdb_mongoapi_resource.go

+	"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
+	"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
+	"github.com/hashicorp/terraform-provider-azurerm/utils"
+	"github.com/jackofallops/kermit/sdk/datafactory/2018-06-01/datafactory"


Is there a reason we're using kermit here rather than go-azure-sdk? It appears supported. We should be avoiding any new usages of Kermit as we are attempting to deprecate and remove this workaround SDK.

Have switched to go-azure-sdk in d95a18f. There are tons of other resources in this folder still using kermit though, I've decided they're out of scope of this PR.

jackofallops · 2024-12-09T08:25:25Z

website/docs/r/data_factory_dataset_cosmosdb_mongoapi.html.markdown

+
+---
+
+* `additional_properties` - (Optional) A map of additional properties to associate with the Data Factory Dataset.


As per parameters, can we make this a little more clear as to what is expected here, and what is supported?

Have added more info to the doc in d95a18f

- Switch to go-azure-sdk - Add nil checks, remove additional_properties from acctest - Document the limited support of string parameters - Renamed resource go type to be consistent with file name

…source

gerrytan · 2024-12-13T01:47:04Z

acctest passed at the latest commit 645977d: acctests_TestAccDataFactoryDatasetCosmosDbMongoDbApi_645977dfbd_20241213-1238+1100.zip

New resource for Data factory CosmosDB MongoDB API dataset

7358021

gerrytan requested a review from a team as a code owner December 5, 2024 01:35

github-actions bot added documentation service/data-factory size/XL waiting-response labels Dec 5, 2024

magodo requested changes Dec 5, 2024

View reviewed changes

gerrytan added 2 commits December 9, 2024 16:04

Merge branch 'main' into gerrytan/datafactory-cosmosdb-mongodb-new-re…

f65b76f

…source

github-actions bot removed the waiting-response label Dec 9, 2024

gerrytan requested a review from magodo December 9, 2024 05:09

jackofallops requested changes Dec 9, 2024

View reviewed changes

github-actions bot added the waiting-response label Dec 9, 2024

rcskosir added the new-resource label Dec 9, 2024

gerrytan added 2 commits December 13, 2024 11:38

Addressing PR feedbacks - iteration 2

d95a18f

- Switch to go-azure-sdk - Add nil checks, remove additional_properties from acctest - Document the limited support of string parameters - Renamed resource go type to be consistent with file name

Merge branch 'main' into gerrytan/datafactory-cosmosdb-mongodb-new-re…

d19f37e

…source

github-actions bot added dependencies and removed waiting-response labels Dec 13, 2024

gerrytan requested a review from jackofallops December 13, 2024 01:19

Renamed variables to be more consistent with other methods

645977d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: azurerm_data_factory_dataset_cosmosdb_mongoapi #28185

New Resource: azurerm_data_factory_dataset_cosmosdb_mongoapi #28185

gerrytan commented Dec 5, 2024

magodo left a comment

magodo commented Dec 9, 2024

gerrytan commented Dec 9, 2024 •

edited

Loading

magodo commented Dec 9, 2024

jackofallops left a comment

jackofallops Dec 9, 2024

gerrytan Dec 13, 2024

jackofallops Dec 9, 2024

gerrytan Dec 13, 2024

jackofallops Dec 9, 2024

gerrytan Dec 13, 2024

jackofallops Dec 9, 2024

gerrytan Dec 13, 2024

gerrytan commented Dec 13, 2024


		* `folder` - (Optional) The folder that this Dataset is in. If not specified, the Dataset will appear at the root level.

		* `parameters` - (Optional) A map of parameters to associate with the Data Factory Dataset.


		---

		* `additional_properties` - (Optional) A map of additional properties to associate with the Data Factory Dataset.

New Resource: azurerm_data_factory_dataset_cosmosdb_mongoapi #28185

Are you sure you want to change the base?

New Resource: azurerm_data_factory_dataset_cosmosdb_mongoapi #28185

Conversation

gerrytan commented Dec 5, 2024

Community Note

Description

PR Checklist

Testing

Change Log

Related Issue(s)

magodo left a comment

Choose a reason for hiding this comment

magodo commented Dec 9, 2024

gerrytan commented Dec 9, 2024 • edited Loading

magodo commented Dec 9, 2024

jackofallops left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gerrytan commented Dec 13, 2024

gerrytan commented Dec 9, 2024 •

edited

Loading