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

Updated README.MD #771

Merged
merged 2 commits into from May 31, 2019
Merged

Updated README.MD #771

merged 2 commits into from May 31, 2019

Conversation

anandvc
Copy link
Contributor

@anandvc anandvc commented Jan 24, 2018

Include the step to create the index for the model that we'll import.

Include the step to create the index for the model that we'll import.
@estolfo
Copy link
Contributor

estolfo commented May 3, 2019

Let's keep the example simple and not include this for now.

@estolfo estolfo closed this May 3, 2019
@anandvc
Copy link
Contributor Author

anandvc commented May 3, 2019

@estolfo Does the example work for you without that line which creates the index?

@estolfo
Copy link
Contributor

estolfo commented May 3, 2019

I was hoping to keep the example as simple as possible and index creation doesn't always happen at the same time as import jobs. Is it really unclear without the create_index! line?

@anandvc
Copy link
Contributor Author

anandvc commented May 3, 2019

@estolfo Yes. The Readme does not mention index creation at all, but it is necessary for the example to work. If you want, we can add a comment for the create_index! line to say that this can be done asynchronously.

@estolfo
Copy link
Contributor

estolfo commented May 6, 2019

@anandvc I think a comment would be better, so index creation right at import time wouldn't be encouraged. I think that operation would usually be done at another point.
I'll reopen the issue if you want to update the PR?

@estolfo estolfo reopened this May 6, 2019
I've added a comment so that users are encouraged to do `create_index!` asynchronously and not at the same time as `import`.
@anandvc
Copy link
Contributor Author

anandvc commented May 30, 2019

Thanks, @estolfo! I've updated the PR and added a comment.

@estolfo
Copy link
Contributor

estolfo commented May 31, 2019

thanks!

@estolfo estolfo merged commit f9e86de into elastic:master May 31, 2019
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.

None yet

2 participants