Skip to content

CDRIVER-4612 document management of search indexes #1366

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

Merged
merged 9 commits into from
Aug 1, 2023

Conversation

kevinAlbs
Copy link
Collaborator

@kevinAlbs kevinAlbs commented Jul 31, 2023

Summary

  • Document management of Atlas Search Indexes.

Background & Motivation

DRIVERS-2637 adds new API to manage Atlas Search Indexes. This PR documents how to run the management commands with existing C driver API.

Rejected idea: Adding API

An API to model the specified Standard API for Search Indexes was considered but rejected.

The Search Indexes helpers do not have special option handling. IMO adding the API would not provide value to wrapping drivers. This PR proposes not adding the API, and instead documents how to manage Search Indexes. If the specified behavior of Search Indexes helpers changes in the future, adding helpers can be reconsidered.

Rejected idea: Adding end-to-end tests to Evergreen

End-to-end (E2E) tests in DRIVERS-2630 were decidedly not added. The proposed C driver changes do not add new API. The E2E tests in Evergreen may not add helpful test coverage. E2E tests require creating an Atlas cluster with setup-atlas-cluster.sh. A local run took 9m28s. I do not think it is worth the effort or maintenance burden.

example-manage-search-indexes was tested manually with an Atlas cluster created with setup-atlas-cluster.sh:

% ./cmake-build/src/libmongoc/example-manage-search-indexes $ATLAS_SEARCH_INDEXES_URI
Created index: "test-index"
Listing indexes:
  { "id" : "64c3c96874355a6d0050331f", "name" : "test-index", "status" : "PENDING", "queryable" : false, "latestDefinition" : { "mappings" : { "dynamic" : false } } }
Updated index: "test-index"
Dropped index: "test-index"

The JSON specification tests are added. The JSON specification tests may be a simpler and lower maintenance test of the documented approach for sending management commands: use of mongoc_collection_command_simple. The JSON specification tests can be removed if desired.

@kevinAlbs kevinAlbs marked this pull request as ready for review July 31, 2023 13:58
if (!mongoc_collection_command_simple (
coll, &cmd, NULL /* read_prefs */, NULL /* reply */, &error)) {
bson_destroy (&cmd);
HANDLE_ERROR ("Failed to run createSearchIndexes: %s", error.message);
Copy link
Contributor

Choose a reason for hiding this comment

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

If this example is run against a non-Atlas server, this example fails on this command attempt and error with a "no such cmd" message. Can we consider adding an earlier "example must be run against Atlas" check and error to make the required conditions a bit more clear?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like that idea. ee78062 adds an earlier check that $listSearchIndexes is supported, and prints an error suggesting the example may not be using Atlas if an error occurred.

Copy link
Contributor

@joshbsiegel joshbsiegel left a comment

Choose a reason for hiding this comment

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

LGTM

@kevinAlbs kevinAlbs merged commit f974c49 into mongodb:master Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants