Skip to content
This repository has been archived by the owner on Feb 5, 2023. It is now read-only.

custom index options? #39

Closed
huglester opened this issue Feb 9, 2017 · 33 comments
Closed

custom index options? #39

huglester opened this issue Feb 9, 2017 · 33 comments

Comments

@huglester
Copy link

Hello

if I would like to have custom index, like:

$params = [
            'index' => $this->index,
            'body' => [
                'settings' => [
                    'number_of_shards' => 5,
                    'number_of_replicas' => 1,
                ],
                'mappings' => [
                    $this->type => [
                        'properties' => [
                            'id' => [
                                'type' => 'integer',
                            ],
                            'uri' => [
                                'type' => 'keyword',
                                //'analyzer' => 'standard'
                            ],
                            'name' => [
                                'type' => 'text',
                                'analyzer' => 'standard'
                            ],
                            'about_me' => [
                                'type' => 'text',
                                'analyzer' => 'standard'
                            ],
                            'created_at' => [
                                'type' => 'date',
                                'format' => 'yyyy-MM-dd HH:mm:ss'
                            ],
                        ],
                    ],
                ],
            ],
        ];

Do you have suggestions?

Thank you!

@thomasjsn
Copy link

Do you mean custom mapping? That you can just create with though the ES API, using something like curl or Postman.

@huglester
Copy link
Author

yeah. Maybe there is some way to 'inject' it when on first usage? or best would be to create custom CLI command?

@roetker
Copy link

roetker commented Feb 10, 2017

For our own usage i added some index/mapping functions to our own fork
https://github.com/Crasmedia/laravel-scout-elastic

Maybe it's interesting for you

@thomasjsn
Copy link

@huglester I liked your suggestion of making it a console command, so that is what I have done for my project: https://gist.github.com/thomasjsn/48185612dc7abe857b9a0ae5716b86c3

I more feel it belongs there than in the search driver.

@ErickTamayo
Copy link
Owner

@thomasjsn that is a nice approach. As you said this is a bit out of scope regarding the search driver. I've made a library for dealing with Elasticsearch but is very outdated as I don't have much time to deal with it: https://github.com/ErickTamayo/Stretchy

Although we can extend this driver to make more Elasticsearch friendly.

@thomasjsn
Copy link

@ErickTamayo I've added a few console commands to my fork that allows for indices and mapping to be easily created. Will you be accepting PRs? Or should I maintain the fork on it's own?

@ErickTamayo
Copy link
Owner

@thomasjsn A PR enhancing the driver will be most than welcome, please be sure to include tests + documentation.

@thomasjsn
Copy link

@ErickTamayo Cool, I have a few more things I'd like to add first. Will update the tests and documentation.

@huglester
Copy link
Author

hello.
thanks on the progress I've been away for a while.

probably best would be to do something like that:
php artisa elastic:reindex App\Post

that way it would take Post model, and look at the:
protected $elasticMapping = []; array or something like that, if no - then 'default' would be used.

@thomasjsn
Copy link

I've added a elastic:make-index {index?} command, that reads the settings and mappings from the elasticsearch config file.

@kkomelin
Copy link

Thank you @roetker and @thomasjsn for your forks. Definitely helpful.

I personally think that it would be great to have mappings attached to each particular model, as it's done in the fork of @roetker, and other search methods, as it's done in @thomasjsn fork.
Can you guys join forces?

And, also, if possible please split different features by different pull requests, for example: 1) Mappings, 2) Sort options, 3) New search methods, etc. This way it would be easier to review and test.

What can I help you?

@thomasjsn
Copy link

thomasjsn commented Feb 20, 2017

@kkomelin Thanks; I can make it so that if mappings are defined in the model they are taken from there, if not; the config file. That should suit us both then :)

I still think the actual creation of the indices and mappings belong in the console command.

Come to think of it; you could do something like this:

        'laravel' => [
            'settings' => [
                "number_of_shards" => 1,
                "number_of_replicas" => 0,
            ],
            'mappings' => [
                'articles' => Article::mapping,
                'comments' => Comment:mapping,
            ]
        ],

@kkomelin
Copy link

kkomelin commented Feb 20, 2017

Thanks @thomasjsn,
I have considered both these approaches of storing mappings, and realized that a model is an abstraction, so maybe it's a good idea to keep Elastic-specific settings out of the model to keep it engine independent. So, I can go with your config approach.

The console command for creating indexes can also work for me.

I'm going to test your fork now. Will share my thoughts if any.

@kkomelin
Copy link

kkomelin commented Feb 20, 2017

@thomasjsn

Come to think of it; you could do something like this:
'articles' => Article::mapping,

Do I need to add "use" for my model classes? If so, I'm wondering whether "php artisan config:cache" works with it?

Anyway, let's maybe focus on your config approach and then if we have time think about mappings in the model.

@thomasjsn
Copy link

No need to add use as long as you put in the full path; I do this in another config file:

'link_classes' => array(
    \Uctrl\Article::class,
)

Config cache works fine.

@thomasjsn
Copy link

thomasjsn commented Feb 20, 2017

@kkomelin My thoughts exactly on the model abstraction. Yes; please test it 👍

I agree with your statement regarding multiple pull requests, I can do some rebasing.

@kkomelin
Copy link

No need to add use as long as you put in the full path; I do this in another config file:
Config cache works fine.

Great. Thanks.

@thomasjsn
Copy link

Hm, maybe I was too quick. I haven't tried calling a static method in the config files before. So I don't really know. You'll need to test it.

@kkomelin
Copy link

@thomasjsn Sure, I will. Going to test your fork now.

@kkomelin
Copy link

@thomasjsn Excellent. The elastic:indexes and elastic:make-index work as expected.
And you suggestion also works:

'mappings' => [
     'articles' => \App\Models\Articles::mappings(),
]

The only thing I have found so far is that the config file doesn't have an empty like at the bottom. Will also test queries and let you know.

@kkomelin
Copy link

kkomelin commented Feb 20, 2017

Ah, one more suggestion, would you mind to rename "elastic:indices" to "elastic:indexes"and "elastic:make-index" to "elastic:make-indexes" (because there can be many)?

As for indices, it's correct but location/area specific http://grammarist.com/usage/indexes-indices/

@thomasjsn
Copy link

@kkomelin Good! What do you mean "an empty like at the bottom"?

I've chosen indices because this is what Elasicsearch is using. Algolia is also using the indices term.

I do agree that the make command should be plural, since it, as you say, can be many.

@kkomelin
Copy link

kkomelin commented Feb 20, 2017

@thomasjsn

@kkomelin Good! What do you mean "an empty like at the bottom"?

Oh, sorry, it's a typo in the word "like". It should be "line". The idea is that any file should have one empty line at the end.

Thanks for clarification regarding "indices". I didn't know that.

@thomasjsn
Copy link

thomasjsn commented Feb 21, 2017

@kkomelin You're right about the line at the bottom, I've set my IDE to always add one 😄

I'll do those changes later today. You asked earlier if you could help, and you can; I would really love some help on documenting it all. After testing you probably have a good idea of how it all works, you could help with the readme file 👍

@kkomelin
Copy link

kkomelin commented Feb 21, 2017

@thomasjsn Thanks.
What do you think is missing in the README? Sorting, mappings inside a model, something else?

@thomasjsn
Copy link

This is what I feel is a bit undocumented at the time:

  • Sorting and mapping from model, as you point out.
  • Query parameters and default in config, how it may be overridden with elasticSearch() method.
  • A few words about the console commands, the optional index argument of make-indices.
  • That the first index defined in the config is used if not searchableWithin() is overridden.

@kkomelin
Copy link

@thomasjsn Thanks.
A bit busy at the moment but I can do that later today. Should I fork your fork and send a PR with the updated documentaiton?

@thomasjsn
Copy link

Yes, that would be great 👍

@kkomelin
Copy link

@thomasjsn Please review my PR with documentation when you have time thomasjsn#3 Thanks.

@thomasjsn
Copy link

@ErickTamayo You want me to rebase before creating pull request?

@ErickTamayo
Copy link
Owner

@thomasjsn I think you can create the PR no need to rebase IMO. I assume you're working on the default branch (3.0). Right now master is outdated.

@thomasjsn
Copy link

@ErickTamayo I've been working on master. Can you get master up to date?

Should it be a new major version? This does implement a lot of changes and the configuration is moved to a dedicated file. So people will need to make som changes before using it. Maybe a 4,0 branch?

@ErickTamayo
Copy link
Owner

Done @thomasjsn. As this has breaking changes it should be a new version.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants