Override ID operations #87
Conversation
ulucinar
changed the title
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
| r.ExternalName = config.NameAsIdentifier | ||
| r.ExternalName.GetExternalNameFn = common.GetNameFromFullyQualifiedID | ||
| // /subscriptions/000-000/resourceGroups/rg1/providers/Microsoft.DocumentDB/databaseAccounts/acc1/sqlDatabases/db1/containers/container1 | ||
| r.ExternalName.GetIDFn = common.GetFullyQualifiedIDFn("Microsoft.DocumentDB", |
There was a problem hiding this comment.
Are we sure we don't want the group of this resource should be cosmosdb instead of documentdb?
There was a problem hiding this comment.
Not sure either. But I believe (though definitely not sure) that cosmodb is a better known name. Also the Terraform counterparts have this prefix.
| @@ -32,6 +38,12 @@ func Configure(p *config.Provider) { | |||
| }, | |||
| } | |||
| r.UseAsync = true | |||
There was a problem hiding this comment.
Do these really need to be async? I know they have long timeouts in Terraform but almost 95% of resources in native providers are sync in the sense that once creation is completed, the resource is ready. Not a big deal though, we can always change it.
There was a problem hiding this comment.
I have not timed them. Not sure how long it takes to provision them. Maybe as we populate examples, we can have rough estimates and switch to synchronous mode for them. Async looks like a safer choice for me.
- SqlContainer -> SQLContainer - SqlDatabase -> SQLDatabase - SqlFunction -> SQLFunction - Move config/virtual into config/network - Fix example manifests - Use default external-name extractor for ResourceGroup references Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
|
Thank you @muvaf! Very much appreciated :) |
Description of your changes
This is a cherry pick of the last two commits of #84.
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested