Update provider to support marqo 2.13 #30

RaynorChavez · 2024-11-06T22:55:33Z

This update adds support for the following fields

treat_urls_and_pointers_as_media
video_preprocessing
audio_preprocessing
model_properties

Within model_properties:

name
dimensions
type
tokens
model_location
url
trust_remote_code
filter_string_max_length

Within model_location:

s3: Has the keys bucket, and key
hf: Has the keys repo_id, and filename
auth_required

ruchira088 · 2024-11-08T07:01:39Z

Adding @orlade since I'm NOT super familiar with Golang

orlade · 2024-11-11T02:55:24Z

docs/data-sources/read_indices.md

+Read-Only:
+
+- `dimensions` (String)
+- `model_location` (String)


I think this should be an object. If the location is well-known, e.g. on HuggingFace, the name can encode it.

orlade · 2024-11-11T02:59:50Z

docs/data-sources/read_indices.md

+<a id="nestedatt--items--model_properties"></a>
+### Nested Schema for `items.model_properties`
+
+Read-Only:


Should include is_marqtuned_model (bool), since we need to specify that if we want a Marqtuned model with custom properties.

There's also authRequired (bool) for private models?

These aren't 2.13 specific but we probably should capture them.

I can't find is_marqtuned_model in the documentation, we should probably ask the marqtune team to add that

Added authRequired thanks

orlade · 2024-11-11T03:03:48Z

docs/resources/index.md

@@ -82,6 +95,20 @@ Optional:
 - `patch_method` (String)


+<a id="nestedatt--settings--model_properties"></a>
+### Nested Schema for `settings.model_properties`


This feels like a duplication; can they both refer to the same schema?

Currently not, the schemas in datasource are all string/bool, whereas there are some Number types in the resource. I tried having datasource refer to the schemas with Number types before but that gave an error.

orlade · 2024-11-11T03:16:19Z

go_marqo/client.go

+	Dimensions      int64  `json:"dimensions"`
+	Type            string `json:"type"`
+	Tokens          int64  `json:"tokens"`
+	ModelLocation   string `json:"model_location"`


Suggested change

ModelLocation string `json:"model_location"`

ModelLocation string `json:"modelLocation"`

orlade · 2024-11-12T00:49:56Z

internal/provider/indices_datasource.go

+	StoreSize                    types.String            `tfsdk:"store_size"`
+	DocsDeleted                  types.String            `tfsdk:"docs_deleted"`
+	SearchQueryTotal             types.String            `tfsdk:"search_query_total"`
+	TreatUrlsAndPointersAsImages types.Bool              `tfsdk:"treat_urls_and_pointers_as_images"`


Should there be a TreatUrlsAndPointersAsMedia?

That's right, added.

orlade · 2024-11-12T00:58:44Z

internal/provider/indices_datasource.go

@@ -390,6 +498,18 @@ func (d *indicesDataSource) Read(ctx context.Context, req datasource.ReadRequest
 			FilterStringMaxLength: types.StringValue(fmt.Sprintf("%d", indexDetail.FilterStringMaxLength)),
 		}

+		// Handle model properties
+		if items[i].ModelProperties.Name.IsNull() &&


Nit: you could make this a method on the ModelProperties struct since it logically belongs to that type:

func (m ModelPropertiesModel) IsEmpty() bool { return m.Name.IsNull() && m.Dimensions.IsNull() && m.Type.IsNull() && m.Tokens.IsNull() && m.Url.IsNull() && m.TrustRemoteCode.IsNull() && m.IsMarqtunedModel.IsNull() }

Thanks, that's cleaner

modified a bit and added

orlade · 2024-11-12T00:59:15Z

internal/provider/indices_datasource.go

 			TextPreprocessing: TextPreprocessingModel{
 				SplitLength:  types.StringValue(fmt.Sprintf("%d", indexDetail.TextPreprocessing.SplitLength)),
 				SplitMethod:  types.StringValue(indexDetail.TextPreprocessing.SplitMethod),
 				SplitOverlap: types.StringValue(fmt.Sprintf("%d", indexDetail.TextPreprocessing.SplitOverlap)),
 			},
-			//ImagePreprocessing: types.ObjectValue(map[string]interface{}, indexDetail.ImagePreprocessing),
+			// ImagePreprocessing


Curious why we don't include this?

Currently the cloud api does not return image_preprocessing (as well as audio and video) in the list index endpoint, so we might need to bring this up with dataplane team

orlade

Not sure if you have a unit test for the "set ModelProperties to empty/nil if its fields are empty" logic, but it might be worthwhile.

orlade · 2024-11-12T01:03:50Z

internal/provider/indices_resource.go

+
+		// Handle model properties
+		if newState.Settings.ModelProperties != nil {
+			if newState.Settings.ModelProperties.Name.ValueString() == "" &&


This logic is different to above where you check IsNull. Should they be the same? If not, why not? Again feels like it could be a method on the struct to make it more obvious what you're actually checking.

I recently changed the resource to refer to objects through a pointer (*ModelPropertiesModelCreate), whereas in datasource it remained a direct struct field.

Logic has the same spirit (just adapted the resource for the datasource)

I've adapted both to use the same struct now

…eturn

RaynorChavez added 12 commits November 4, 2024 15:37

init changes - new fields

6b9ea73

init working audio and video fields

95773fe

Add modelproperties dict to datasource

fb8b7bb

Add support for modelProperties and trustRemoteCode

573cb34

rename tests for staging

0989709

run go generate

898f695

Add go unit tests

f01d846

change examples

322d765

add type to client test returns

535248b

add new resource integration tests

2b43426

remove separate unit test step add more client unit tests

75a885e

change test

06ead2e

RaynorChavez requested a review from ruchira088 November 8, 2024 05:52

ruchira088 requested a review from orlade November 8, 2024 07:01

RaynorChavez added 2 commits November 8, 2024 18:08

cleanup

9bc27b5

cleanup

591d4a2

orlade requested changes Nov 11, 2024

View reviewed changes

RaynorChavez added 4 commits November 11, 2024 15:24

address comments

601c1fb

run generate

9fa81e3

address comments and attempt tests fix

e32cfa1

attempt test fix

7cc77f8

orlade requested changes Nov 12, 2024

View reviewed changes

RaynorChavez added 3 commits November 12, 2024 12:51

attempt test fix

3ecaaf7

fix ModelProperties conversion issue

809c9a9

linting

99be041

orlade approved these changes Nov 12, 2024

View reviewed changes

RaynorChavez added 2 commits November 12, 2024 16:51

fix bugs and tests

c2f1383

fix unit tests

477faef

RaynorChavez added 3 commits November 12, 2024 17:36

Merge branch 'main' into raynor/update_to_mq213

dabae34

remove stray image_preprocessing in languagebind update config

53ed373

fix audio and video preprocessing validates against nonexistent api r…

7e395b3

…eturn

orlade approved these changes Nov 13, 2024

View reviewed changes

RaynorChavez merged commit 4e31c20 into main Nov 13, 2024
3 checks passed

RaynorChavez deleted the raynor/update_to_mq213 branch November 13, 2024 04:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update provider to support marqo 2.13 #30

Update provider to support marqo 2.13 #30

RaynorChavez commented Nov 6, 2024 •

edited

Loading

ruchira088 commented Nov 8, 2024 •

edited

Loading

orlade Nov 11, 2024

RaynorChavez Nov 11, 2024

orlade Nov 11, 2024

orlade Nov 11, 2024

RaynorChavez Nov 11, 2024 •

edited

Loading

RaynorChavez Nov 11, 2024

orlade Nov 11, 2024

RaynorChavez Nov 11, 2024 •

edited

Loading

orlade Nov 11, 2024

RaynorChavez Nov 11, 2024

orlade Nov 12, 2024

RaynorChavez Nov 12, 2024

orlade Nov 12, 2024

RaynorChavez Nov 12, 2024

RaynorChavez Nov 12, 2024

orlade Nov 12, 2024

RaynorChavez Nov 12, 2024

orlade left a comment

orlade Nov 12, 2024

RaynorChavez Nov 12, 2024 •

edited

Loading

	ModelLocation string `json:"model_location"`
	ModelLocation string `json:"modelLocation"`

Update provider to support marqo 2.13 #30

Update provider to support marqo 2.13 #30

Conversation

RaynorChavez commented Nov 6, 2024 • edited Loading

ruchira088 commented Nov 8, 2024 • 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

RaynorChavez Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RaynorChavez Nov 11, 2024 • 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orlade left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RaynorChavez Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

RaynorChavez commented Nov 6, 2024 •

edited

Loading

ruchira088 commented Nov 8, 2024 •

edited

Loading

RaynorChavez Nov 11, 2024 •

edited

Loading

RaynorChavez Nov 11, 2024 •

edited

Loading

RaynorChavez Nov 12, 2024 •

edited

Loading