Add mappings and bulk to quickstart page #2417
Conversation
|
A documentation preview will be available soon.
Request a new doc build by commenting
If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here. |
There was a problem hiding this comment.
This looks great, thanks! Only a few more details to iron out.
Did you know that you could see the result of your changes here? https://elasticsearch-py--2417.org.readthedocs.build/en/2417/quickstart.html
docs/sphinx/quickstart.rst
Outdated
| mappings = { | ||
| "properties": { | ||
| "foo": { | ||
| "type" : "text" | ||
| }, | ||
| "bar" : { | ||
| "type" : "text", | ||
| "fields" : { | ||
| "keyword" : { | ||
| "type" : "keyword", | ||
| "ignore_above" : 256 | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you please format the code using https://github.com/psf/black to fix the indentation? You may need a comma after 256 to get the results you want.
docs/sphinx/quickstart.rst
Outdated
| @@ -57,7 +85,7 @@ This is how you create the `my_index` index: | |||
|
|
|||
| .. code-block:: python | |||
|
|
|||
| client.indices.create(index="my_index") | |||
| client.indices.create(index="my_index", mappings = mappings) | |||
There was a problem hiding this comment.
Ideally all code snippets should be directly runnable, what do you think about defining mappings in the same code block?
There was a problem hiding this comment.
This is a quickstart, I think there should be only one way to create an index, what do you think?
I would then remove "Creating an index" and rename "Create a mapping for your index" to "Create an index with mappings"
docs/sphinx/quickstart.rst
Outdated
|
|
||
| def generate_docs(documents, index_name): | ||
| for i, document in enumerate(documents): | ||
| yield dict(_index=index_name, _id=f"{i}", _source=document) |
There was a problem hiding this comment.
This is equivalent to yield {"_index": index_name, "_id": f"{i}", "_source": document}. Is there a specific reason to prefer using dict?
docs/sphinx/quickstart.rst
Outdated
| for i, document in enumerate(documents): | ||
| yield dict(_index=index_name, _id=f"{i}", _source=document) | ||
|
|
||
| helpers.bulk(client, generate_docs(books, index_name)) |
There was a problem hiding this comment.
As with the mapping, this code is not runnable. Maybe generate_docs should be defined like that?
def generate_docs():
for i in range(10):
yield {
"_index": "my_index",
"foo": f"foo {i}",
"bar": "bar",
}
helpers.bulk(client, generate_docs())The advantages of this version:
- It can be copy/pasted directly
- It reuses the index created above
- It's easy to adapt to generate much more documents
- It does not specify a doc id, which is better for performance
docs/sphinx/quickstart.rst
Outdated
|
|
||
| helpers.bulk(client, generate_docs(books, index_name)) | ||
|
|
||
| These helpers are the recommended simple and streamlined way to abstract otherwise complicated and verbose functions such as `client.bulk`. |
There was a problem hiding this comment.
Unlike Markdown, you need two backticks in reStructuredText to format as code: single backticks only add emphasis. What do you think of this wording?
| These helpers are the recommended simple and streamlined way to abstract otherwise complicated and verbose functions such as `client.bulk`. | |
| These helpers are the recommended way to perform bulk ingestion. While it is also possible to perform bulk ingestion using ``client.bulk`` directly, the helpers handle retries, ingesting chunk by chunk and more. See the :ref:`helpers` page for more details. |
I've considered linking to the client.bulk() docs using :meth:~elasticsearch.Elasticsearch.bulk` but I did not find it clearer as it's only rendered as "bulk()" with a link to the docs.
docs/sphinx/quickstart.rst
Outdated
| @@ -57,7 +85,7 @@ This is how you create the `my_index` index: | |||
|
|
|||
| .. code-block:: python | |||
|
|
|||
| client.indices.create(index="my_index") | |||
| client.indices.create(index="my_index", mappings = mappings) | |||
There was a problem hiding this comment.
This is a quickstart, I think there should be only one way to create an index, what do you think?
I would then remove "Creating an index" and rename "Create a mapping for your index" to "Create an index with mappings"
docs/sphinx/quickstart.rst
Outdated
| } | ||
| } | ||
|
|
||
| client.indices.create(index="my_index", mappings = mappings) |
There was a problem hiding this comment.
| client.indices.create(index="my_index", mappings = mappings) | |
| client.indices.create(index="my_index", mappings=mappings) |
removing the double index instructoins
pquentin
changed the title
(cherry picked from commit a844537)
Added a few more examples including dealing with mappings and indexing multiple documents at once (in two alternative ways).
These changes have also been added to the 00 notebook in search-labs, that is also linked to under
interactive examples.Response to issue #2139 requesting changes to documentation