Open damienalexandre opened 1 year ago
Currently the factory allows to override multiple things with these options:
public const CONFIG_BULK_SIZE = 'elastically_bulk_size';
public const CONFIG_DENORMALIZER = 'elastically_denormalizer';
public const CONFIG_INDEX_CLASS_MAPPING = 'elastically_index_class_mapping';
public const CONFIG_INDEX_NAME_MAPPER = 'elastically_index_name_mapper';
public const CONFIG_INDEX_PREFIX = 'elastically_index_prefix';
public const CONFIG_MAPPINGS_DIRECTORY = 'elastically_mappings_directory';
public const CONFIG_MAPPINGS_PROVIDER = 'elastically_mappings_provider';
public const CONFIG_SERIALIZER = 'elastically_serializer';
public const CONFIG_SERIALIZER_CONTEXT_BUILDER = 'elastically_serializer_context_builder';
public const CONFIG_SERIALIZER_CONTEXT_PER_CLASS = 'elastically_serializer_context_per_class';
Am I wrong or some of them cannot be by the elastically configuration so far ? I only see config for:
bulk_size
index_class_mapping
prefix
mapping_directory
mapping_provider_service
serializer. context_builder_service
serializer. context_mapping
So we're missing:
denormalizer
index_name_mapper
serializer
I think you are right :+1:
I don't think we need an open issue for such things.
Instead, let's do like Symfony: We keep an UPGRADE.md file up to date to track all deprecation. Then when the 2.x branch is created, we drop everything.
I don't think we need an open issue for such things.
Instead, let's do like Symfony: We keep an UPGRADE.md file up to date to track all deprecation. Then when the 2.x branch is created, we drop everything.
I may be wrong but to me this issue is more than just "Remove deprecated code", it's "Let's deprecate the Factory".
1) Currently the factory class is not deprecated. 2) As explained by my previous message, currently the factory class cannot be deprecated since everything is not allowed by the config.
So the step to resolve this issue would be:
serializer_service
optiondenormalizer_service
optionindex_name_mapper
option@VincentLanglet :
Currently the factory class is not deprecated.
I agree, let's deprecate it completely
As explained by my previous message, currently the factory class cannot be deprecated since everything is not allowed by the config.
We don't need bundle configuration for overriding a service. I'm a bit against adding options to override any part of the lib.
For exemple, the bulk_size
, or the mapping_directly
are values depending on the application structure / design.
The denormalizer
, serializer
, index_name_mapper
are usually not changed. So we don't need an entry in the configuration to change them.
For example, in Symfony, you don't have an option to change the serializer / router, etc. if you want to do so, you decorate the existing one, or you replace it.
As explained by my previous message, currently the factory class cannot be deprecated since everything is not allowed by the config.
We don't need bundle configuration for overriding a service. I'm a bit against adding options to override any part of the lib.
For exemple, the
bulk_size
, or themapping_directly
are values depending on the application structure / design.The
denormalizer
,serializer
,index_name_mapper
are usually not changed. So we don't need an entry in the configuration to change them.For example, in Symfony, you don't have an option to change the serializer / router, etc. if you want to do so, you decorate the existing one, or you replace it.
Sure, I was expecting such answer. Maybe I missing some point about services injections/decorations.
The definition of the Indexer is done this way in Elastically:
->set('elastically.abstract.indexer', Indexer::class)
->abstract()
->args([
'$client' => service(Client::class),
'$serializer' => service('serializer'),
'$bulkMaxSize' => abstract_arg('bulk size'),
'$bulkRequestParams' => [],
'$contextBuilder' => abstract_arg('elastically.abstract.static_context_builder'),
])
If I decorate the serializer
services:
serializer:
class: App\NewSerializer
it will override the serializer for the whole application, not just for the elastically services.
What if I only want to change the serializer used by Elastically services ?
you need to override the service used in elastically:
service:
elastically.my_connection.indexer:
class: App\NewIndexer
Dependency Injection should be the only way to deal with all our services.
For 2.0, let's get rid of all the error prone code and have only one way to use the lib - removing the Factory.