feat: Supports change_stream_options_pre_and_post_images_expire_after_seconds in `mongodbatlas_cluster` and `mongodbatlas_advanced_cluster` #2528

oarbusi · 2024-08-27T13:06:58Z

Description

Supports change_stream_options_pre_and_post_images_expire_after_seconds in mongodbatlas_cluster
Link to any related issue(s): CLOUDP-262696

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals I have added appropriate changelog entries.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

…s in cluster

AgustinBettati

overall approach LGTM, had some small doubts

internal/service/cluster/model_cluster.go

AgustinBettati · 2024-08-28T14:36:12Z

internal/service/cluster/resource_cluster_test.go

@@ -206,6 +208,7 @@ func TestAccCluster_basic_DefaultWriteRead_AdvancedConf(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "advanced_configuration.0.oplog_size_mb", "1000"),
 					resource.TestCheckResourceAttr(resourceName, "advanced_configuration.0.sample_refresh_interval_bi_connector", "310"),
 					resource.TestCheckResourceAttr(resourceName, "advanced_configuration.0.sample_size_bi_connector", "110"),
+					resource.TestCheckResourceAttr(resourceName, "advanced_configuration.0.change_stream_options_pre_and_post_images_expire_after_seconds", "-1"),


nice test scenario

docs/resources/cluster.md

internal/service/advancedcluster/model_advanced_cluster.go

This reverts commit d5b690f.

This reverts commit 5d1fef4.

github-actions · 2024-09-06T16:05:51Z

APIx bot: a message has been sent to Docs Slack channel

EspenAlbert · 2024-09-10T07:42:55Z

internal/service/advancedcluster/model_advanced_cluster.go

-func flattenProcessArgs(p *admin20240530.ClusterDescriptionProcessArgs) []map[string]any {
-	if p == nil {
+func flattenProcessArgs(p20240530 *admin20240530.ClusterDescriptionProcessArgs, p *admin.ClusterDescriptionProcessArgs20240805) []map[string]any {
+	if p20240530 == nil {


names looks strange to me. I would go for a more generic: pOld and pNew

I did it like this to be consistent on how we handle different SDK version and also thinking in the future where we might only use the "new", so the refactor would be cleaner and we would only keep the p (there wouldn't be a need to change from pNew to p)

EspenAlbert · 2024-09-10T07:45:49Z

internal/service/advancedcluster/resource_advanced_cluster.go

+	if err != nil {
+		return diag.FromErr(fmt.Errorf(errorConfigRead, clusterName, err))
+	}
+	processArgs, _, err := connV2.ClustersApi.GetClusterAdvancedConfiguration(ctx, projectID, clusterName).Execute()


Why do we need both processArgs20240530 and processArgs? Will the GetClusterAdvancedConfiguration ever return nil?

ChangeStreamOptionsPreAndPostImagesExpireAfterSeconds is only available in processArgs which is from the latest SDK versrion

Can we not remove the processArgs20240530?

AgustinBettati

LGTM, just a final doubt on my end

AgustinBettati · 2024-09-10T07:46:53Z

internal/service/advancedcluster/model_advanced_cluster.go

+				"change_stream_options_pre_and_post_images_expire_after_seconds": {
+					Type:     schema.TypeInt,
+					Optional: true,
+					Default:  -1,


Just to confirm, was the final agreement with the API team that we send the default of -1 if attribute is not defined?

-1 and null(not set) is equivalent in this case for the API. Setting the default value to -1 in the schema is necessary for the case of the user wanting to go back to the default value(which might not always be the same) after having set a different value. This would means that the user would remove the attribute from the TF config, and if there's not default value, it wouldn't be updated in the server because if the value is null in a PATCH, it's not sent

EspenAlbert

Nice. Some small nits. Also, got a bit confused about the schema for advanced configuration only lives in the advanced_cluster.

oarbusi · 2024-09-10T09:43:18Z

Merging. I ran the local build of the provider and advanced_cluster and cluster works as expected

* master: doc: Adds 1.19.0 release upgrade guide (#2564) chore: Updates CHANGELOG.md for #2528 feat: Supports change_stream_options_pre_and_post_images_expire_after_seconds in `mongodbatlas_cluster` and `mongodbatlas_advanced_cluster` (#2528) chore: Disables preview mode for EAR private endpoint so it may be normally accessible (#2571) doc: Adds support for SDK_BRANCH in schema generation (#2562) update git workflow (#2572) chore: Updates CHANGELOG.md for #2566 feat: Adds `mongodbatlas_stream_processor` resource and data sources (#2566) chore: Updates CHANGELOG.md for #2569 chore: Merges Azure KMS Encryption at Rest Private Endpoint feature to master (#2569) chore: Updates CHANGELOG.md for #2568 doc: Includes sync_creation into mongodbatlas_online_archive resource documentation (#2567) fix: Sets correct `zone_id` when `use_replication_spec_per_shard` is false and refactors `replica_set_scaling_strategy` handling with old schema of advanced cluster (#2568) chore: Updates CHANGELOG.md for #2539 feat: Support `replica_set_scaling_strategy` in `mongodbatlas_advanced_cluster` (#2539) adding changelog entry for 1.18.1 to avoid confusion (#2561) # Conflicts: # internal/provider/provider.go

support change_stream_options_pre_and_post_images_expire_after_second…

1798d48

…s in cluster

github-actions bot added the enhancement label Aug 27, 2024

oarbusi added 7 commits August 27, 2024 15:09

changelog entry

e8806e4

fix changelog entry

0c06f7b

check valid value

e1ac7ee

fix flattener

90c9ff4

use getok

ecff0a9

default value -1(off) so it goes to default value when not setting

ffd33db

add to attribute to DS model

b118d22

AgustinBettati reviewed Aug 28, 2024

View reviewed changes

EspenAlbert reviewed Aug 28, 2024

View reviewed changes

internal/service/advancedcluster/model_advanced_cluster.go Show resolved Hide resolved

oarbusi added 17 commits August 28, 2024 17:43

set default in data source

171e32d

specify default value of attribute in doc

cbf0843

Merge branch 'master' into CLOUDP-262696

bdec336

remove value validation, letting API fail

411ef2f

temp: do not run advanced_cluster test

5d1fef4

temp:remove advanced cluster in CI

d5b690f

Merge branch 'master' into CLOUDP-262696

57b2158

Merge branch 'master' into CLOUDP-262696

405688f

Revert "temp:remove advanced cluster in CI"

e97989d

This reverts commit d5b690f.

Revert "temp: do not run advanced_cluster test"

128c4a2

This reverts commit 5d1fef4.

implement in advanced_cluster

8acfdea

fix pointer

3eab2bd

docs

743076b

changelog

f713c5e

skip migration test (new attribute has default value)

7e6d821

skip mig

fdf246a

skip mig

1891238

oarbusi changed the title ~~feat: Supports change_stream_options_pre_and_post_images_expire_after_seconds in mongodbatlas_cluster~~ feat: Supports change_stream_options_pre_and_post_images_expire_after_seconds in mongodbatlas_cluster and mongodbatlas_advanced_cluster Sep 6, 2024

use processArgs instead of attribute

ba4caa0

oarbusi marked this pull request as ready for review September 6, 2024 16:05

oarbusi requested review from a team as code owners September 6, 2024 16:05

davidhou17 approved these changes Sep 6, 2024

View reviewed changes

run tests

ee5b254

EspenAlbert reviewed Sep 10, 2024

View reviewed changes

AgustinBettati approved these changes Sep 10, 2024

View reviewed changes

EspenAlbert approved these changes Sep 10, 2024

View reviewed changes

oarbusi merged commit 03c37f3 into master Sep 10, 2024
33 of 35 checks passed

oarbusi deleted the CLOUDP-262696 branch September 10, 2024 09:43

svc-apix-Bot added a commit that referenced this pull request Sep 10, 2024

chore: Updates CHANGELOG.md for #2528

1c0b244

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Supports change_stream_options_pre_and_post_images_expire_after_seconds in `mongodbatlas_cluster` and `mongodbatlas_advanced_cluster` #2528

feat: Supports change_stream_options_pre_and_post_images_expire_after_seconds in `mongodbatlas_cluster` and `mongodbatlas_advanced_cluster` #2528

oarbusi commented Aug 27, 2024

AgustinBettati left a comment

AgustinBettati Aug 28, 2024

github-actions bot commented Sep 6, 2024

EspenAlbert Sep 10, 2024

oarbusi Sep 10, 2024

EspenAlbert Sep 10, 2024

oarbusi Sep 10, 2024

EspenAlbert Sep 10, 2024

AgustinBettati left a comment

AgustinBettati Sep 10, 2024

oarbusi Sep 10, 2024 •

edited

Loading

EspenAlbert left a comment

oarbusi commented Sep 10, 2024

feat: Supports change_stream_options_pre_and_post_images_expire_after_seconds in mongodbatlas_cluster and mongodbatlas_advanced_cluster #2528

feat: Supports change_stream_options_pre_and_post_images_expire_after_seconds in mongodbatlas_cluster and mongodbatlas_advanced_cluster #2528

Conversation

oarbusi commented Aug 27, 2024

Description

Type of change:

Required Checklist:

Further comments

AgustinBettati left a comment

Choose a reason for hiding this comment

AgustinBettati Aug 28, 2024

Choose a reason for hiding this comment

github-actions bot commented Sep 6, 2024

EspenAlbert Sep 10, 2024

Choose a reason for hiding this comment

oarbusi Sep 10, 2024

Choose a reason for hiding this comment

EspenAlbert Sep 10, 2024

Choose a reason for hiding this comment

oarbusi Sep 10, 2024

Choose a reason for hiding this comment

EspenAlbert Sep 10, 2024

Choose a reason for hiding this comment

AgustinBettati left a comment

Choose a reason for hiding this comment

AgustinBettati Sep 10, 2024

Choose a reason for hiding this comment

oarbusi Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

EspenAlbert left a comment

Choose a reason for hiding this comment

oarbusi commented Sep 10, 2024

feat: Supports change_stream_options_pre_and_post_images_expire_after_seconds in `mongodbatlas_cluster` and `mongodbatlas_advanced_cluster` #2528

feat: Supports change_stream_options_pre_and_post_images_expire_after_seconds in `mongodbatlas_cluster` and `mongodbatlas_advanced_cluster` #2528

oarbusi Sep 10, 2024 •

edited

Loading