> Luckily, it doesn't look like we ship the SLF4J API in our runtime
binaries, so we already have a situation where downstream projects can
choose the SLF4J version of both the API and provider Jars.

Ryan, this is a good point. Yes, our Spark and Flink runtime bundle jars
exclude slf4j, which is a good thing.

If a downstream project/application has dependency on iceberg modules (like
api/core), slf4j-api 2.0 can be propagated transitively from the next
Iceberg 1.7 release during version resolution. But downstream can always
explicitly align the versions of slf4j api and provider jars as best
practice. We can call out the slf4j 2.0 change in Iceberg 1.7 release notes
though.

If the community is ok with the slf4j-api 2.0 upgrade, please approve the
PR. It would also fix the broken test logging (due to api and provider not
aligned for Iceberg test code)
https://github.com/apache/iceberg/pull/11001

Thanks,
Steven


On Mon, Sep 16, 2024 at 11:50 AM Russell Spitzer <russell.spit...@gmail.com>
wrote:

> Sounds reasonable to me to just go to 2.x
>
> On Mon, Sep 16, 2024 at 1:10 PM rdb...@gmail.com <rdb...@gmail.com> wrote:
>
>> If I understand the SLF4J announcement correctly, it sounds like the best
>> option is to rely on binary compatibility between the 1.x and 2.x clients.
>>
>> As long as we don't use the newer API, then the compiled code can use
>> either a 1.7.x or 2.0.x API Jar. The API Jar needs to match the provider
>> version, so it should be supplied by downstream code instead of Iceberg.
>> Luckily, it doesn't look like we ship the SLF4J API in our runtime
>> binaries, so we already have a situation where downstream projects can
>> choose the SLF4J version of both the API and provider Jars.
>>
>> In that case, I think the best path forward is to upgrade to 2.x but not
>> use the new API features that will cause problems if downstream libraries
>> are not already on 2.x.
>>
>> Does that sound reasonable?
>>
>> On Wed, Sep 11, 2024 at 11:17 AM Steven Wu <stevenz...@gmail.com> wrote:
>>
>>> Following up on the discussion from the community sync meeting. Right
>>> now, Iceberg test code is in the 3rd row in the table pasted below. With
>>> the recent Avro 1.12 upgrade (and slf4j 2.x), the main code is also
>>> affected. That means downstream applications (Spark, Trino, Flink, ...) may
>>> run into the same situation when upgrading to the next Iceberg 1.7 release.
>>>
>>> [image: image.png]
>>>
>>> From the slf4j doc, it seems that *slf4j 2.x API is backward compatible
>>> as long as provider/binding is updated to 2.x too*.
>>> https://www.slf4j.org/faq.html#changesInVersion200
>>>
>>> We have two options
>>> 1. Exclude the slf4j 2.x transitive dependency from Avro 1.12. but we
>>> would need to be comprehensive on forcing the slf4j dependency version
>>> resolution to 1.x everywhere until Iceberg is ready to move forward to
>>> slf4j 2.x.
>>> 2. Move forward with slf4j 2.x now with a clear callout in the 1.7
>>> release notes.
>>>
>>> First option is the more conservative approach and defer the slf4j 2.x
>>> upgrade decision to the downstream applications. But it will add a little
>>> burden to make sure future dependency upgrades don't bring in slf4j 2.x
>>> transitively. Maybe we can leverage the force resolution from Gradle?
>>>
>>> configurations.all {
>>>   resolutionStrategy {
>>>     force 'org.slf4j:slf4j-api:1.7.35'
>>>   }
>>> }
>>>
>>>
>>> Thanks,
>>> Steven
>>>
>>> SLF4J 2.0 stable release was announced *2 years ago*:
>>> https://mailman.qos.ch/pipermail/announce/2022/000176.html. It
>>> explained binary compatibility.
>>>
>>> "Mixing different versions of slf4j-api.jar and SLF4J provider can
>>> cause problems. For example, if you are usingslf4j-api-2.0.0.jar, then you
>>> should also use slf4j-simple-2.0.0.jar, using slf4j-simple-1.5.5.jar will
>>> not work."
>>>
>>> More notes from slf4j FAQ page.
>>>
>>> "SLF4J 2.0.0 incorporates an optional fluent api
>>> <https://www.slf4j.org/manual.html#fluent>. Otherwise, there are no
>>> client facing API changes in 2.0.x. For most users, upgrading to version
>>> 2.0..x should be a drop-in replacement, as long as the logging provider is
>>> updated as well.
>>>
>>> From the clients perspective, the SLF4J API, more specifically the
>>> org.slf4j package, is backward compatible for all versions. This means
>>> that you can upgrade from SLF4J version 1.0 to any later version without
>>> problems. Code compiled with *slf4j-api-versionN.jar* will work with
>>> *slf4j-api-versionM.jar* for any versionN and any versionM. *To date,
>>> binary compatibility in slf4j-api has never been broken."*
>>>
>>>
>>> On Mon, Sep 9, 2024 at 9:22 AM Steven Wu <stevenz...@gmail.com> wrote:
>>>
>>>> Bump the thread to bring the awareness of the issue and implication of
>>>> slf4j 2.x upgrade.
>>>>
>>>> On Mon, Aug 26, 2024 at 12:24 PM Steve Zhang
>>>> <hongyue_zh...@apple.com.invalid> wrote:
>>>>
>>>>> I believe dependabot tried to upgrade self4j to 2.x in [1] but JB
>>>>> mentioned there's -1 on this upgrade, maybe he has more context.
>>>>>
>>>>> [1]https://github.com/apache/iceberg/pull/9688
>>>>>
>>>>> Thanks,
>>>>> Steve Zhang
>>>>>
>>>>>
>>>>>
>>>>> On Aug 24, 2024, at 7:37 PM, Steven Wu <stevenz...@gmail.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> It seems that test logging is broken in many modules (like core,
>>>>> flink) because slf4j-api was upgraded to 2.x while slf4j-simple provider 
>>>>> is
>>>>> still on 1.7. I created a PR that upgraded slf4j-simple testImplementation
>>>>> to 2.x for all subprojects.
>>>>>
>>>>> https://github.com/apache/iceberg/pull/11001
>>>>>
>>>>> That fixed the test logging problem (e.g. TestInMemoryCatalog). You
>>>>> can find more details in the PR description. Test logging seems to have
>>>>> been broken for a while (from 1.4). But those dep problems have been for 
>>>>> *test
>>>>> runtime only*.
>>>>>
>>>>> Recent change [1] on Avro 1.12.0 introduced slf4j-api 2.x change for
>>>>> runtime, as verified by the cmd below on the *main branch*.
>>>>>
>>>>> ./gradlew -q :iceberg-core:dependencyInsight --dependency slf4j-api
>>>>> --configuration runtimeClasspath
>>>>>
>>>>> This thread is to raise awareness on the slf4j-api dep change to 2.x
>>>>> as downstream projects/applications can be affected. Looking forward to
>>>>> feedback on the path forward.
>>>>>
>>>>>    1. continue the current upgrade path and document the slf4j-api
>>>>>    2.x change in the next 1.7 release. But we need to be cautious of not
>>>>>    porting the Avro 1.12.0 / slf4j-api 2.x change to the 1.6 branch.
>>>>>    2. exclude slf4j-api transitive deps from Avro and other test deps
>>>>>    so that Iceberg stays on slf4j 1.7. If that is the direction to go, we
>>>>>    won't need  PR #11001.
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Steven
>>>>>
>>>>> [1] https://github.com/apache/iceberg/pull/10879
>>>>>
>>>>>
>>>>>

Reply via email to