> On May 30, 2022, at 1:41 PM, Volkan Yazıcı <[email protected]> wrote:
>
> On Mon, May 30, 2022 at 6:52 PM Ralph Goers <[email protected]>
> wrote:
>
>>> On May 30, 2022, at 2:01 AM, Volkan Yazıcı <[email protected]> wrote:
>>> Following up on the responses, I will ask another question: In 3.x, can't
>>> we deprecate either key or name and stick to only one?
>>
>> Why? What problem does this solve?
>>
>>> For instance, I am
>>> in favor of keeping only `name`, adding a compile-time check to validate
>>> its content, and cloning it in a `key` field (marked as to-be-removed in
>>> future) for backward compatibility. The bottom line is, I think if we
>> _can_
>>> live with one, we should.
>>
>> This is a typical Plugin definition.
>>
>> @Plugin(name = "Failover", category = Core.CATEGORY_NAME, elementType =
>> Appender.ELEMENT_TYPE, printObject = true)
>>
>> So you want to remove the elementType? Note that “name” is used as the
>> key to store the plugin into its Category Map. The elementType is usually
>> what is in the “name" attribute but some Plugins, such as Route, don’t
>> have an elementType. In those cases the name attribute of the entry is set
>> to the plugin’s name attribute. PluginRegistry creates a PluginType which
>> includes the name attribute. This is used all over the place for various
>> reasons.
>> So I don’t see how either key or name can be removed since they are used
>> for completely different purposes.
>>
>
> I did not propose anywhere to make a change on or remove `elementType`, I
> think we have a misunderstanding. My point is simple, `key` can be used
> anywhere `name` is used right now. Am I mistaken with this assessment? Is
> there any usage of `name` where if it is replaced by the contents of `key`
> the functionality gets broken?
Yes, you are mistaken in you assessment. I guess I must not have been clear in
what I wrote above. For all practical purposes the “name” attribute and
elementType
are the same thing. Only when an element type is not declared does the “name”
attribute have a different value. However, the key always has the value of the
plugin
name attribute.
So no, they are not the same and no, one cannot be replaced with the other.
>
>
>>> Yes, but it completes an important refactoring Matt has been tackling:
>>> plugin simplification. The frontend (user-facing) part of this story is
>>> implemented; `@Configurable` and `@Plugin` are two separate concerns.
>>> Though the backend is still the same: the entire `@Plugin` metadata model
>>> contains properties of `@Configurable`.
>>
>> Maybe so, but Matt didn’t actually remove anything. IMO breaking up
>> PluginEntry
>> wouldn’t solve anything and would, in fact, make the code more complicated.
>>
>
> I simply don't understand why we store every plugin as if it is something
> to be read from a file-based configuration. If that is so, let's revert
> Matt's changes and make sure every plugin must be configurable. If not,
> let's reflect that in the code. `PluginEntry` shouldn't care about the
> concerns of configurations. It is the configuration logic that needs to
> figure out whether a particular `PluginEntry` implements
> `ConfigurablePluginEntry` interface or not and acts accordingly.
Only “Core” plugins are directly concerned about the Configuration. Admittedly,
that is over 80% of the plugins.
However, all the PatternConverters are driven by the pattern attribute wherever
it is used. So that is also essentially a file-based configuration. We also
have
Lookups. But Lookups are located based on a Lookup pattern being specified
somewhere in the configuration, so I guess they are file-based too. We also
have Watcher plugins. Those are chosen based on the configuration file
location String, so those are configuration based too.
In short, the reason they are all tied to “file-based” configuration is because
the
whole point of plugins is to convert some textual configuration data into a set
of
methods that implement specific interfaces.
Again, to me it feels like you are trying to “fix” something that isn’t broken.
Ralph
