Remove ConfigCompatibility

[manofthepeace]

Out of curiosity I started bulldozing a ton of things that were deprecated for a while or before the lts. Then I figured maybe baby steps are better. I am also not sure this is wanted at all. If so please let me know I'll drop the deprecation cleanup I was playing with.

Ci green in my fork with this one. This was changed in 3.10.

[radcortez]

/cc @dmlloyd

[dmlloyd]

I don't think we're quite ready to remove this yet, since the corresponding config was changed fairly recently - that would have to be up to @maxandersen and/or @cescoffier. Also the change should be made in the class itself on a per property basis, since it could be used to manage many configuration name changes over time.

[cescoffier]

In general, we try to apply the "2 LTS" versions. If the method was deprecated in 3.10, it was not yet released in an LTS (the previous was 3.8, and the next is 3.15).

[gsmet]

@cescoffier where does this 2 LTS rule come from? Because I always enforced having the deprecated in at least 1 LTS (meaning that it would be seen by people only using LTS).
Given they have 6 months until the next LTS, they have plenty of time to handle the deprecations.

[cescoffier]

That's the same rule, we just use different names for it.

[cescoffier]

I mean it need to be deprecated in a least on LTS version - which is not the case here.

[gsmet]

@cescoffier well, I'm not sure it's the same. Because it will be deprecated in 3.14/3.15, we won't backport this change.

So the deprecation will be in one LTS (3.15) and dropped in the next one (when the 3.16 will end up in an LTS).

[cescoffier]

Yes, it could be done in 3.16 - and you are right, it's the current main branch.

[maxandersen]

Sorry for my ignorance but are we removing the interceptor that will automatically handle renames and print warning to now do nothing at all?

Is there really an overhead in keeping this around making it more likely that users can update easily ?

I'm all for removing hard to maintain, or somehow performance affecting deprecations but this here seem to be of little value to remove while keeping it makes it easier to upgrade for users ?

Or am I missing something ?

[radcortez]

Is there really an overhead in keeping this around making it more likely that users can update easily ?

There is always an overhead, which depends on how you run things (native vs jvm, number of available sources, etc). Having these pieces will increase the number of lookups in the Config system.

As a reference, look here for REST Client (it is extreme):
#42932 (comment)

We could have to look up 12 different names to populate one property. This is noticeable, especially with the native image.

In this particular case, using only a handful of names with a single rule should not cause a significant overhead.

[dmlloyd]

Note that this additional lookup only affects properties that were renamed, not all properties. Also, I believe that all of the properties currently found in ConfigCompatibility are build-time properties, so native image should not be affected at all I think.

The main consideration should be: Are the individual renames no longer desired? If so, the individual renames could be removed, otherwise they should stay. But I'd keep the class because AFAIK it's the first and only fully correct mechanism to relocate a config property after changing its name - which is something that I would expect we may want to do again in the future.

[cescoffier]

So, what should we do here?

[radcortez]

I think we should keep it for a little longer.

[dmlloyd]

I'll just reiterate what I said before:

The main consideration should be: Are the individual renames no longer desired? If so, the individual renames could be removed, otherwise they should stay. But I'd keep the class because AFAIK it's the first and only fully correct mechanism to relocate a config property after changing its name - which is something that I would expect we may want to do again in the future.

I'd keep the class even if there are zero names in it. I'd be quite surprised if we never need to relocate names in the future.

[cescoffier]

Ok - closing the PR. See comments

(Thanks anyway, always appreciated!)

[gsmet]

Given the job was done, let's keep it as draft and marked on ice. This way we won't lose the branch.