-
Notifications
You must be signed in to change notification settings - Fork 10
DOCSP-51348-atlas-search-page #122
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
DOCSP-51348-atlas-search-page #122
Conversation
✅ Deploy Preview for docs-kotlin-sync ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🔄 Deploy Preview for docs-kotlin-sync processing
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with some small comments!
source/atlas-search.txt
Outdated
This aggregation pipeline operator is only available for collections hosted | ||
on :atlas:`MongoDB Atlas </>` clusters running v4.2 or later that are | ||
covered by an :atlas:`Atlas search index </reference/atlas-search/index-definitions/>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[s] Looking at the Atlas Search Overview page, it seems like when this is phrased "Atlas Search index" the "S" is capitalized but if it's just "search index" then it's lowercase.
covered by an :atlas:`Atlas search index </reference/atlas-search/index-definitions/>`. | |
covered by an :atlas:`Atlas Search index </reference/atlas-search/index-definitions/>`. |
source/atlas-search.txt
Outdated
on :atlas:`$searchMeta </atlas-search/query-syntax/#-searchmeta>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[q] Does this page contain version availability? I didn't see it at the linked page. This one has a little note about sharded collection compatibility but that's it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, i took this from the java docs. can remove!
source/atlas-search.txt
Outdated
on :atlas:`$searchMeta </atlas-search/query-syntax/#-searchmeta>`. | ||
|
||
The following example shows the ``near`` metadata for an Atlas search |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following example shows the ``near`` metadata for an Atlas search | |
The following example shows the ``near`` metadata for an Atlas Search |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall great; with some minor issues to consider improving on.
source/atlas-search.txt
Outdated
Use the ``searchMeta()`` method to create a :manual:`$searchMeta | ||
</reference/operator/aggregation/searchMeta/>` pipeline stage, which returns | ||
only the metadata from of the Atlas full-text search results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from of?
val settings = MongoClientSettings.builder() | ||
.applyConnectionString(ConnectionString(uri)) | ||
.retryWrites(true) | ||
.build() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems it will align better with existing pattern to connection uri placeholder in code directly without indirection of a constant field:
val uri = "<connection string>"
val settings = MongoClientSettings.builder()
.applyConnectionString(ConnectionString(uri))
.retryWrites(true)
.build()
|
||
// Uncomment the methods that correspond to what you're testing | ||
// runAtlasTextSearchMeta(collection) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need the above comment? For one thing, it is not methods to be uncommented for the statement below is a method invocation, not the method declaration per se; so Uncomment the below statement
is better?
But given we have provided this method as example, I think simply using it directly makes more sense:
runAtlasTextSearchMeta(collection)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true, this is a remnant of old code where there were multiple methods that could be tested 😅 good catch!
source/includes/atlas-search.kt
Outdated
val database = mongoClient.getDatabase("sample_mflix") | ||
val collection = database.getCollection<Document>("movies") | ||
|
||
// Queries for documents that have a "title" value containing the word "Alabama" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the above comment might not be 100% accurate, for text
operator relies on some analyzer so it allows for some extent of fuzzy matching. For instance, if the title contains word ALABAMA
, it could be returned as well. Also, the "title" here is a field, so the verbiage is far from accurate.
A better one might be:
Full-text search on "title" field using query "Alabama"
Seems the code has been self-explanatory so no additional comment is needed?
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-51348
Staging Links
Self-Review Checklist