Skip to content

Update ElasticsearchProviderResourceMetadataCollectionFactory.php #5264

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

Closed
wants to merge 1 commit into from

Conversation

rapaelector
Copy link

by default the elasticsearch is not false but null so it will create confusion with indices and try to check the index as the shortName. But the shortName by default is the model class and it is not true for every cases , even if the index is different of the class and create an issue

Q A
Branch? main for features / current stable version branch for bug fixes
Tickets #...
License MIT
Doc PR api-platform/docs#...

@rapaelector
Copy link
Author

rapaelector commented Dec 12, 2022

I see that if we custom the index like this :

api_platform:
    ....
    mapping:
        paths:
            - '%kernel.project_dir%/src/Model'
    elasticsearch:
        hosts: [ '%env(ELASTICSEARCH_HOST)%' ]
        mapping:
            App\Model\Document:
                index: '%env(APP_ENV)%_hubble_document'
                type: actor

we have to specify in the Model the provider and the elasticsearch :

use ApiPlatform\Elasticsearch\State\ItemProvider;
.......
#[Get(normalizationContext: ['groups' => ['actor-read']], elasticsearch: false, provider: ItemProvider::class)]

But why it is not automatic to found the provider and why should I add the elasticsearch as false ?

@soyuka
Copy link
Member

soyuka commented Dec 12, 2022

Right, can you target 3.0 ? We've an ongoing work on elasticsearch here : https://github.com/api-platform/core/pull/5213/files

@rapaelector
Copy link
Author

Right, can you target 3.0 ? We've an ongoing work on elasticsearch here : https://github.com/api-platform/core/pull/5213/files

thanks for your reply, yes I’ll do it tomorrow

@soyuka
Copy link
Member

soyuka commented Dec 16, 2022

supersseeded by #5272 thanks!

@soyuka soyuka closed this Dec 16, 2022
@soyuka
Copy link
Member

soyuka commented Dec 16, 2022

There are tests failing over that: #5272

Indeed, I think there's more to it as if elasticsearch is enabled we want to try and get indices. This is quite bad DX we need to improve it but its not yet a priority. please patch this over to your codebase by overriding the service for now or help me get the tests to work at 5272 thanks!

@rapaelector
Copy link
Author

thanks for your reply ,
I've sent you this PR that will fix the test : 51

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.

3 participants