feat: Adds type and fields attributes in resource and data sources for search_index #1605

lantoli · 2023-11-07T08:13:48Z

Description

Jira ticket: INTMDB-1260

Adds type and fields attributes in resource and data sources for search_index.

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.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the 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
If changes include deprecations or removals, I defined an isolated PR with a relevant title as it will be used in the auto-generated changelog.

Further comments

mongodbatlas/resource_mongodbatlas_search_index.go

lantoli · 2023-11-13T14:49:47Z

website/docs/r/search_index.html.markdown

-### Advanced (with custom analyzers)
+### Basic vector index
+```terraform
+resource "mongodbatlas_search_index" "test-basic-search-vector" {


@Zuhairahmed do we want a different example?

@lantoli suggest for time being we leverage example that Atlas Search team has provided here for fields parameter: https://www.mongodb.com/docs/atlas/atlas-search/field-types/knn-vector/#std-label-fts-data-types-knn-vector

"fields": { "plot_embedding": { "type": "knnVector", "dimensions": 1536, "similarity": "euclidean" } }

also believe you are missing index name parameter which is required:

to prevent confusion with cluster_name and collection_name, suggest we call this index_name

thanks. your example won't work directly but adapting the idea. current "name" attribute is the index name, that doesn't change.

AgustinBettati · 2023-11-13T15:23:57Z

mongodbatlas/resource_mongodbatlas_search_index.go

@@ -272,6 +293,10 @@ func resourceMongoDBAtlasSearchIndexRead(ctx context.Context, d *schema.Resource
 		return diag.Errorf("error setting `index_id` for search index (%s): %s", d.Id(), err)
 	}

+	if err := d.Set("type", searchIndex.Type); err != nil {


Does the API always responds with a value for the type field? Would consider marking as computed or defining a default value in the schema to avoid inconsistencies in the state.

if the index was created withouth type, it won't return type. API responds same as in creation so I won't use computed.
as it's defined as optional, it has default value = "" (zero value for strings), this is what we want. When refreshing the state the "type" attribute is added to the state with "" but no plan change is shown to the users.

suggest to have this question/answer represented with a test, not necessarily blocking this PR (unless it's trivial creating it).

kyuan-mongodb

LGTM!

marcosuma

it would be good to add migration and acceptance tests to this change

mongodbatlas/data_source_mongodbatlas_search_index.go

mongodbatlas/data_source_mongodbatlas_search_indexes.go

lantoli · 2023-11-14T07:16:03Z

it would be good to add migration and acceptance tests to this change

@marcosuma I added an acceptance test and planning to add more. About migration (and unit) I think the idea is doing it in the new FW resources, this is not migrated yet.

I did manual tests that showed a change in TF state (from unexisting Type and Fields to Type="", Fields=nil) but didn't trigger a plan change to the user.

lantoli · 2023-11-14T07:17:04Z

mongodbatlas/resource_mongodbatlas_search_index_test.go

@@ -156,6 +175,45 @@ func TestAccSearchIndexRS_importBasic(t *testing.T) {
 	})
 }

+func TestAccSearchIndexRS_withVector(t *testing.T) {


@marcosuma FYI basic acceptance test, planning to add more

…ime for them to be deleted

formatting

github-actions · 2023-11-16T10:46:47Z

Package	Line Rate	Health
github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas	2%	❌
github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas/framework/validator	68%	➖
github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas/testutils	37%	❌
github.com/mongodb/terraform-provider-mongodbatlas/mongodbatlas/util	17%	❌
Summary	3% (279 / 10376)	❌

marcosuma · 2023-11-16T12:54:50Z

mongodbatlas/resource_mongodbatlas_search_index.go

@@ -272,6 +293,10 @@ func resourceMongoDBAtlasSearchIndexRead(ctx context.Context, d *schema.Resource
 		return diag.Errorf("error setting `index_id` for search index (%s): %s", d.Id(), err)
 	}

+	if err := d.Set("type", searchIndex.Type); err != nil {


suggest to have this question/answer represented with a test, not necessarily blocking this PR (unless it's trivial creating it).

lantoli · 2023-11-16T13:14:21Z

mongodbatlas/resource_mongodbatlas_search_index_test.go

+	})
+}
+
+func TestAccSearchIndexRS_withSearchType(t *testing.T) {


@marcosuma FYI this tests checks that you can explicitly pass "search" as index type.

lantoli · 2023-11-16T13:14:58Z

mongodbatlas/resource_mongodbatlas_search_index_test.go

+					resource.TestCheckResourceAttr(datasourceName, "mappings_dynamic", "true"),
+					resource.TestCheckResourceAttr(datasourceName, "search_analyzer", searchAnalyzer),
+					resource.TestCheckResourceAttr(datasourceName, "name", indexName),
+					resource.TestCheckResourceAttr(datasourceName, "type", ""),


@marcosuma FYI this tests checks that type is empty when created in that way

AgustinBettati

LGTM, just a small detail

AgustinBettati · 2023-11-16T13:33:26Z

mongodbatlas/resource_mongodbatlas_search_index.go

+			Type:     schema.TypeString,
+			Optional: true,
+		},
+		"fields": {


have we considered adding a conflictsWith validation to ensure vectorSearch attributes are not defined together with regular search attributes?

at the end I preferred that all those checks are done in the server. If we do them in client-side there is some risk that both logics differ now or in the future.

lantoli changed the title ~~feat: Add Update resource search_index~~ feat: Adds type and fields attributes in resource search_index Nov 7, 2023

lantoli changed the title ~~feat: Adds type and fields attributes in resource search_index~~ feat: Adds type and fields attributes in resource and data sources for search_index Nov 8, 2023

lantoli force-pushed the INTMDB-1260_resource_index branch from caeba29 to e457758 Compare November 9, 2023 13:00

tibulca reviewed Nov 13, 2023

View reviewed changes

mongodbatlas/resource_mongodbatlas_search_index.go Show resolved Hide resolved

lantoli commented Nov 13, 2023

View reviewed changes

AgustinBettati reviewed Nov 13, 2023

View reviewed changes

kyuan-mongodb approved these changes Nov 13, 2023

View reviewed changes

marcosuma reviewed Nov 14, 2023

View reviewed changes

mongodbatlas/data_source_mongodbatlas_search_index.go Show resolved Hide resolved

marcosuma reviewed Nov 14, 2023

View reviewed changes

mongodbatlas/data_source_mongodbatlas_search_indexes.go Show resolved Hide resolved

lantoli commented Nov 14, 2023

View reviewed changes

lantoli added 19 commits November 16, 2023 11:31

add type field

1306c0b

add fields field

a79554d

log when using existing cluster

b050b0f

remove skip

3180d68

change random prefixes

2b19e79

fix tests

e3bb1e5

const collectionName

1d69ae4

const searchAnalyzer

02b2b92

const resourceName and datasourceName

46a83d3

reduce cluster name size

a615529

don't check dangling indices for existing clusters as it takes some t…

62125d8

…ime for them to be deleted

failing test for vector search index

e8fa159

create vector index in test

71a8a7c

change data source

90980ac

continue if it can't unmarshal

98a5bf3

add fields to search-indexes data source

f50782b

initial doc

1cf1f42

small fix in doc

1f88258

fix example

c5f6174

Zuhairahmed and others added 8 commits November 16, 2023 11:31

Update search_index.html.markdown

347f18e

formatting

Update search_index.html.markdown

262218d

formatting

adapt doc and example for Fields

8dd9c8b

detect format errors in fields and mappings_fields

d673dd4

detect format errors in analyzers

06df642

test stringified JSON attributes for fields

267d30e

generic json diff check

e31705a

test with explicit search type

45fa280

lantoli force-pushed the INTMDB-1260_resource_index branch from 0e415bf to 45fa280 Compare November 16, 2023 10:34

lantoli marked this pull request as ready for review November 16, 2023 10:38

lantoli requested a review from a team as a code owner November 16, 2023 10:38

marcosuma approved these changes Nov 16, 2023

View reviewed changes

lantoli commented Nov 16, 2023

View reviewed changes

AgustinBettati approved these changes Nov 16, 2023

View reviewed changes

lantoli merged commit 921c7b2 into master Nov 16, 2023

lantoli deleted the INTMDB-1260_resource_index branch November 16, 2023 13:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Adds type and fields attributes in resource and data sources for search_index #1605

feat: Adds type and fields attributes in resource and data sources for search_index #1605

lantoli commented Nov 7, 2023 •

edited

Loading

lantoli Nov 13, 2023

Zuhairahmed Nov 15, 2023 •

edited

Loading

Zuhairahmed Nov 15, 2023

Zuhairahmed Nov 15, 2023 •

edited

Loading

lantoli Nov 15, 2023

Zuhairahmed Nov 15, 2023

AgustinBettati Nov 13, 2023

lantoli Nov 13, 2023 •

edited

Loading

marcosuma Nov 16, 2023

kyuan-mongodb left a comment

marcosuma left a comment

lantoli commented Nov 14, 2023

lantoli Nov 14, 2023

github-actions bot commented Nov 16, 2023

marcosuma Nov 16, 2023

lantoli Nov 16, 2023

lantoli Nov 16, 2023

AgustinBettati left a comment

AgustinBettati Nov 16, 2023

lantoli Nov 16, 2023

feat: Adds type and fields attributes in resource and data sources for search_index #1605

feat: Adds type and fields attributes in resource and data sources for search_index #1605

Conversation

lantoli commented Nov 7, 2023 • edited Loading

Description

Type of change:

Required Checklist:

Further comments

Choose a reason for hiding this comment

Zuhairahmed Nov 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zuhairahmed Nov 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lantoli Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyuan-mongodb left a comment

Choose a reason for hiding this comment

marcosuma left a comment

Choose a reason for hiding this comment

lantoli commented Nov 14, 2023

Choose a reason for hiding this comment

github-actions bot commented Nov 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AgustinBettati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lantoli commented Nov 7, 2023 •

edited

Loading

Zuhairahmed Nov 15, 2023 •

edited

Loading

Zuhairahmed Nov 15, 2023 •

edited

Loading

lantoli Nov 13, 2023 •

edited

Loading