Skip to content
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

Make the README use a single type in examples. #26098

Merged
merged 3 commits into from
Aug 10, 2017

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Aug 8, 2017

It currently indexes both users and tweets as their own types.

Closes #25978

It currently indexes both users and tweets as their own types.

Closes elastic#25978
@jpountz jpountz added >bug >docs General docs changes v6.0.0 labels Aug 8, 2017
Copy link
Member

@dadoonet dadoonet left a comment

Choose a reason for hiding this comment

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

It looks good to me but I think we should may be not promote using different type of data within the same index.

WDYT?

README.textile Outdated
@@ -50,17 +49,19 @@ h3. Indexing
Let's try and index some twitter like information. First, let's create a twitter user, and add some tweets (the @twitter@ index will be created automatically):

<pre>
curl -XPUT 'http://localhost:9200/twitter/user/kimchy?pretty' -H 'Content-Type: application/json' -d '{ "name" : "Shay Banon" }'
curl -XPUT 'http://localhost:9200/twitter/doc/user_kimchy?pretty' -H 'Content-Type: application/json' -d '{ "type": "user", "name" : "Shay Banon" }'
Copy link
Member

@dadoonet dadoonet Aug 8, 2017

Choose a reason for hiding this comment

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

In real life, would we create instead 2 indices?

curl -XPUT 'http://localhost:9200/twitter_user/doc/kimchy'
curl -XPUT 'http://localhost:9200/twitter_tweet/doc/1'

@jpountz
Copy link
Contributor Author

jpountz commented Aug 8, 2017

I did not do that in order to keep changes minimal but I think you are right that we should use different indices for tweets and users, I'll fix.

@jpountz
Copy link
Contributor Author

jpountz commented Aug 8, 2017

@dadoonet I ended up only having one index for tweets to keep things simple, can you have another look?

Copy link
Member

@dadoonet dadoonet left a comment

Choose a reason for hiding this comment

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

Left some other comments.

README.textile Outdated
@@ -125,29 +129,27 @@ h3. Multi Tenant - Indices and Types

Man, that twitter index might get big (in this case, index size == valuation). Let's see if we can structure our twitter system a bit differently in order to support such large amounts of data.

Elasticsearch supports multiple indices, as well as multiple types per index. In the previous example we used an index called @twitter@, with two types, @user@ and @tweet@.
Elasticsearch supports multiple indices, and each index may store heterogeneous documents. In the previous example we used an index called @twitter@ that stored both @user@s and @tweet@s.
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to say that in this intro documentation?

Copy link
Member

Choose a reason for hiding this comment

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

"that stored both" is wrong now.

{
"user": "kimchy",
"post_date": "2009-11-15T14:12:12",
"message": "Another tweet, will it be indexed?"
}'
</pre>

The above will index information into the @kimchy@ index, with two types, @info@ and @tweet@. Each user will get their own special index.
The above will index information into the @kimchy@ index. Each user will get their own special index.
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe this is true but I might be misreading though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks ok to me?

Copy link
Member

Choose a reason for hiding this comment

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

My bad.

Copy link
Member

@dadoonet dadoonet left a comment

Choose a reason for hiding this comment

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

LGTM!

{
"user": "kimchy",
"post_date": "2009-11-15T14:12:12",
"message": "Another tweet, will it be indexed?"
}'
</pre>

The above will index information into the @kimchy@ index, with two types, @info@ and @tweet@. Each user will get their own special index.
The above will index information into the @kimchy@ index. Each user will get their own special index.
Copy link
Member

Choose a reason for hiding this comment

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

My bad.

@jpountz jpountz merged commit 4e27edd into elastic:master Aug 10, 2017
@jpountz jpountz deleted the fix/README branch August 10, 2017 10:27
jpountz added a commit that referenced this pull request Aug 10, 2017
It currently indexes both users and tweets as their own types.

Closes #25978
jpountz added a commit that referenced this pull request Aug 10, 2017
It currently indexes both users and tweets as their own types.

Closes #25978
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Aug 14, 2017
* master: (30 commits)
  Rewrite range queries with open bounds to exists query (elastic#26160)
  Fix eclipse compilation problem (elastic#26170)
  Epoch millis and second formats parse float implicitly (Closes elastic#14641) (elastic#26119)
  fix SplitProcessor targetField test (elastic#26178)
  Fixed typo in README.textile (elastic#26168)
  Fix incorrect class name in deleteByQuery docs (elastic#26151)
  Move more token filters to analysis-common module
  reindex: automatically choose the number of slices (elastic#26030)
  Fix serialization of the `_all` field. (elastic#26143)
  percolator: Hint what clauses are important in a conjunction query based on fields
  Remove unused Netty-related settings (elastic#26161)
  Remove SimpleQueryStringIT#testPhraseQueryOnFieldWithNoPositions.
  Tests: reenable ShardReduceIT#testIpRange.
  Allow `ClusterState.Custom` to be created on initial cluster states (elastic#26144)
  Teach the build about betas and rcs (elastic#26066)
  Fix wrong header level
  inner hits: Unfiltered nested source should keep its full path
  Document how to import Lucene Snapshot libs when elasticsearch clients (elastic#26113)
  Use `global_ordinals_hash` execution mode when sorting by sub aggregations. (elastic#26014)
  Make the README use a single type in examples. (elastic#26098)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes v6.0.0-beta2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants