Skip to content

INTMDB-251: Update search rs and ds to use go-client v0.12.0 #548

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Aug 31, 2021
Merged

INTMDB-251: Update search rs and ds to use go-client v0.12.0 #548

merged 4 commits into from
Aug 31, 2021

Conversation

abner-dou
Copy link
Contributor

Description

Search Index Resource and Data Source was updated in order to use the new go-client release (v0.12.0)

Link to any related issue(s):

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Documentation fix/enhancement

Required Checklist:

  • I have signed the MongoDB CLA
  • I have read the Terraform contribution guidelines
  • 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

Further comments

Tests Results:

-----------------------------------------------------
--- PASS: TestAccResourceMongoDBAtlasSearchIndex_withMapping (589.53s)
PASS
coverage: 8.4% of statements
ok      github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas 592.030s        coverage: 8.4% of statements
?       github.com/mongodb/terraform-provider-mongodbatlas/version      [no test files]


--- PASS: TestAccResourceMongoDBAtlasSearchIndex_basic (701.34s)
PASS
coverage: 7.7% of statements
ok      github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas 703.770s        coverage: 7.7% of statements
?       github.com/mongodb/terraform-provider-mongodbatlas/version      [no test files]

gssbzn
gssbzn reviewed Aug 31, 2021
View reviewed changes
go.sum
Comment on lines +980 to +981
go.mongodb.org/atlas v0.12.0 h1:/vnHX3rh8jdPrP8mRznuU/2VrGH+cCdz8/Esrzpvaus=
go.mongodb.org/atlas v0.12.0/go.mod h1:wVCnHcm/7/IfTjEB6K8K35PLG70yGz8BdkRwX0oK9/M=
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] please run go mod tidy to clean the old version from this file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I think it's still there because of the realm client... we can look at that on a different PR

gssbzn
gssbzn reviewed Aug 31, 2021
View reviewed changes
Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this, there are some linting erros and also I left a little suggestion to clean the dependency file

mongodbatlas/resource_mongodbatlas_search_index.go Outdated
Optional: true,
Elem: customAnalyzersSchema(),
//Elem: customAnalyzersSchema(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we delete this if not needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, I totally forgot to remove it

mongodbatlas/resource_mongodbatlas_search_index.go Outdated
@@ -48,9 +47,10 @@ func returnSearchIndexSchema() map[string]*schema.Schema {
Required: true,
},
"analyzers": {
Type: schema.TypeList,
Type: schema.TypeString,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

making sure I get this right, analyzers and field mapping for terradform are now "strings" of json that we just map to the go client interface{} type correct? general question nothing blcoking here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right!
Now the analyzers field will be like the mapping_fields field, both are JSON now (string)

themantissa
themantissa requested changes Aug 31, 2021
View reviewed changes
Copy link
Collaborator

@themantissa themantissa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - just need to address Gustavo's comments and add one thing. Can you add a short upgrade guide to note the breaking changes? Just want to make sure we call those out (basically it's the changes you made in the docs. Once we have that we'll move forward quickly! Thank you!

gssbzn
gssbzn approved these changes Aug 31, 2021
View reviewed changes
Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

themantissa
themantissa approved these changes Aug 31, 2021
View reviewed changes
Copy link
Collaborator

@themantissa themantissa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for quick turn around! LGTM!

@themantissa themantissa merged commit 8a48c35 into master Aug 31, 2021
@themantissa themantissa linked an issue Aug 31, 2021 that may be closed by this pull request
Error: error getting search index information: json: cannot unmarshal array into Go struct field IndexMapping.mappings.fields of type mongodbatlas.IndexField #545
Closed
@thetonymaster thetonymaster deleted the INTMDB-251 branch September 15, 2021 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gssbzn gssbzn gssbzn approved these changes

@themantissa themantissa themantissa approved these changes

@thetonymaster thetonymaster thetonymaster approved these changes

@coderGo93 coderGo93 Awaiting requested review from coderGo93

@andreaangiolillo andreaangiolillo Awaiting requested review from andreaangiolillo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@abner-dou @gssbzn @themantissa @thetonymaster