-
Notifications
You must be signed in to change notification settings - Fork 178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Uses use_replication_spec_per_shard
in data sources
#2896
Conversation
a88025a
to
5d80ddc
Compare
) | ||
if overrideUsingLegacySchema { | ||
legacySchema = true | ||
zoneNameNumShards = numShardsMapFromOldAPI(clusterRespOld) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plan is empty in Read when overriding legacy, so we get it from the old API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this comment to the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added in: d39cf2f
@@ -37,13 +39,21 @@ func findNumShardsUpdates(ctx context.Context, state, plan *TFModel, diags *diag | |||
return planCounts | |||
} | |||
|
|||
func resolveAPIInfo(ctx context.Context, plan *TFModel, diags *diag.Diagnostics, clusterLatest *admin.ClusterDescription20240805, client *config.MongoDBClient) *ExtraAPIInfo { | |||
func resolveAPIInfo(ctx context.Context, diags *diag.Diagnostics, client *config.MongoDBClient, plan *TFModel, clusterLatest *admin.ClusterDescription20240805, overrideUsingLegacySchema bool) *ExtraAPIInfo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] forceLegacySchema
less verbose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed in: d39cf2f
asymmetricShardUnsupported := false | ||
|
||
api20240530 := client.AtlasV220240530.ClustersApi | ||
clusterRespOld, _, err := api20240530.GetCluster(ctx, projectID, clusterName).Execute() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactor!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. If we merge this now, we don't have to undo changes later here in this PR, we'll have to merge master to that branch
AsymmetricShardUnsupported: asymmetricShardUnsupported, | ||
} | ||
if forceLegacySchema { | ||
info.UsingLegacySchema = true | ||
info.ZoneNameNumShards = numShardsMapFromOldAPI(clusterRespOld) // plan is empty in data source Read when forcing legacy, so we get num_shards from the old API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[q] can you elaborate on what you mean by "plan is empty" here? Simply that num_shards won't have a value or something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in a data source the plan is empty, it only has the attributes provided by the user in the TF files. for example replication_specs will be empty
Description
Uses
use_replication_spec_per_shard
in data sourcesLink to any related issue(s): CLOUDP-290307
Type of change:
Required Checklist:
Further comments