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

Update tutorial-load-dataset.asciidoc #11703

Merged
merged 6 commits into from
May 12, 2017

Conversation

bhavyarm
Copy link
Contributor

This PR is for fixing load sample data into kibana website for 6.0.0_alpha1 https://www.elastic.co/guide/en/kibana/master/tutorial-load-dataset.html

I have updated the mapping for string -> keyword.

1.Do we change the schema on the page to reflect ES mappings? Specifically do I change INT to long? and all the strings into text fields? According to this?
https://www.elastic.co/guide/en/elasticsearch/reference/master/mapping.html#mapping

2.Do I remove the "default" : from the curl command to create mapping?

The data files for logs and accounts.zip are fine. I do need to change the shakespeare.json and pass it to Deb to upload it.

@bhavyarm bhavyarm requested review from jbudz and epixa May 10, 2017 17:27
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

Minor comment below, looking good

@@ -78,7 +78,7 @@ curl -H 'Content-Type: application/json' -XPUT http://localhost:9200/shakespeare

This mapping specifies the following qualities for the data set:

* The _speaker_ field is a string that isn't analyzed. The string in this field is treated as a single unit, even if
* The _speaker_ field is a keyword that isn't analyzed. The text in this field is treated as a single unit, even if
Copy link
Member

Choose a reason for hiding this comment

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

thoughts on changing this to ...field that isn't analyzed? I think the not analyzed portion overlaps with keyword.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. That makes sense to me.

@@ -78,7 +78,7 @@ curl -H 'Content-Type: application/json' -XPUT http://localhost:9200/shakespeare

This mapping specifies the following qualities for the data set:

* The _speaker_ field is a string that isn't analyzed. The string in this field is treated as a single unit, even if
* The _speaker_ field is a field that isn't analyzed. The text in this field is treated as a single unit, even if
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'd flip this around and say, Because the speaker and play_name fields are keyword fields, they are not analyzed. The strings are treated as a single unit even if they contain multiple words." and drop "The same applies to the play_name field."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense to me.

@debadair
Copy link
Contributor

I think the schema is ok as-is. Not sure about default, it might be better to specify a single type?

It would be nice if these curl examples were tested console examples instead.

Incorporating Deb's comments to change curl commands as console tested
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

The copy as curl option in the built docs doesn't have content-type, example:
edit: disregard, I was using an old version of the docs builder


You should see output similar to the following:

[source,shell]
[source,js]
Copy link
Member

Choose a reason for hiding this comment

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

any reason why this one was changed to [source,js]?

Using curl command to display indices created
@bhavyarm
Copy link
Contributor Author

@jbudz shall I go back to curl for those commands? Thanks!

bhavyarm added 2 commits May 11, 2017 16:36
Using shell and console tested command to get the _cat/indices
@jbudz
Copy link
Member

jbudz commented May 11, 2017

I realize now why we're using _default_ with the shakespeare mappings - there are two types with some overlapping field names.

@bhavyarm bhavyarm merged commit d7ce103 into elastic:master May 12, 2017
bhavyarm added a commit that referenced this pull request May 12, 2017
Changing the tutorial to update it according to the new ES mappings and adding console tested commands.
bhavyarm added a commit that referenced this pull request May 12, 2017
Changing the tutorial to update it according to the new ES mappings and adding console tested commands.
bhavyarm added a commit that referenced this pull request May 12, 2017
Changing the tutorial to update it according to the new ES mappings and adding console tested commands.
bhavyarm added a commit that referenced this pull request May 12, 2017
Changing the tutorial to update it according to the new ES mappings and adding console tested commands.
bhavyarm added a commit that referenced this pull request May 12, 2017
Changing the tutorial to update it according to the new ES mappings and adding console tested commands.
bhavyarm added a commit that referenced this pull request May 12, 2017
Changing the tutorial to update it according to the new ES mappings and adding console tested commands.
snide pushed a commit to snide/kibana that referenced this pull request May 30, 2017
Changing the tutorial to update it according to the new ES mappings and adding console tested commands.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants