New data source azurerm_communication_service
#22426
Conversation
myc2h6o
force-pushed
the
communication_service_data
branch
from
dc53f7a to
58f3374
Compare
|
test result |
There was a problem hiding this comment.
hey @myc2h6o
Thanks for this PR - I've taken a look through and left some comments inline, but if we can fix those up then this should otherwise be good to go 👍
Thanks!
|
|
||
| keysResp, err := client.ListKeys(ctx, id) | ||
| if err != nil { | ||
| return fmt.Errorf("listing keys for %s: %+v", id, err) |
There was a problem hiding this comment.
data sources should gracefully handle having limited permissions, so we shouldn't raise this error when there's a permissions error (presumably a 401?)
| if model.PrimaryConnectionString != nil { | ||
| state.PrimaryConnectionString = *model.PrimaryConnectionString | ||
| } | ||
|
|
||
| if model.PrimaryKey != nil { | ||
| state.PrimaryKey = *model.PrimaryKey | ||
| } | ||
|
|
||
| if model.SecondaryConnectionString != nil { | ||
| state.SecondaryConnectionString = *model.SecondaryConnectionString | ||
| } | ||
|
|
||
| if model.SecondaryKey != nil { | ||
| state.SecondaryKey = *model.SecondaryKey | ||
| } |
There was a problem hiding this comment.
FWIW we can reduce this to:
| if model.PrimaryConnectionString != nil { | |
| state.PrimaryConnectionString = *model.PrimaryConnectionString | |
| } | |
| if model.PrimaryKey != nil { | |
| state.PrimaryKey = *model.PrimaryKey | |
| } | |
| if model.SecondaryConnectionString != nil { | |
| state.SecondaryConnectionString = *model.SecondaryConnectionString | |
| } | |
| if model.SecondaryKey != nil { | |
| state.SecondaryKey = *model.SecondaryKey | |
| } | |
| state.PrimaryConnectionString = pointer.From(model.PrimaryConnectionString) | |
| state.PrimaryKey = pointer.From(model.PrimaryKey) | |
| state.SecondaryConnectionString = pointer.From(model.SecondaryConnectionString) | |
| state.SecondaryKey = pointer.From(model.SecondaryKey) |
|
@tombuildsstuff thanks for reviewing! I've updated the code to use |
|
updated test result |
|
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. |
Close #21736