Yesterday we reverted the PR in main and cherry picked into the 1.7.x
branch https://github.com/apache/iceberg/pull/11817

Today I have opened https://github.com/apache/iceberg/pull/11830 which is
the previous code with some new tests and a fix for the breaking change

On Wed, Dec 18, 2024 at 1:59 PM Marc Cenac <marc.ce...@datadoghq.com> wrote:

> Apologies for introducing the breaking change.  I just opened
> https://github.com/apache/iceberg/pull/11812 to revert.  Let me know if
> there is something else I can do to help fix
>
> On Tue, Dec 17, 2024 at 8:35 AM Jean-Baptiste Onofré <j...@nanthrax.net>
> wrote:
>
>> Hi Cheng,
>>
>> Thanks for the update. The issue appeared in the test, but I guess it
>> can also impact runtime use.
>>
>> Let me take a look at the update PR.
>>
>> Regards
>> JB
>>
>> On Tue, Dec 17, 2024 at 3:06 PM Cheng Pan <pan3...@gmail.com> wrote:
>> >
>> > Spark 3.5.4 (under RC3 voting) introduces a breaking change that
>> requires Iceberg to be patched[1]. Please include Spark 3.5.4 support in
>> the patch release.
>> >
>> > Since some users are sticky to Java 8, it would be great if Iceberg
>> also releases a new patch version of 1.6.x, otherwise they won’t be able to
>> use the latest Spark version combined with Iceberg.
>> >
>> > [1] https://github.com/apache/iceberg/pull/11731
>> >
>> > Thanks,
>> > Cheng Pan
>> >
>> >
>> >
>> > On Dec 17, 2024, at 17:58, Jean-Baptiste Onofré <j...@nanthrax.net>
>> wrote:
>> >
>> > That works for me. In the meantime, I will draft a proposal (in terms
>> > of content) for 1.8.0.
>> >
>> > I volunteer to drive 1.7.2 release if needed.
>> >
>> > Regards
>> > JB
>> >
>> > On Tue, Dec 17, 2024 at 9:58 AM Fokko Driesprong <fo...@apache.org>
>> wrote:
>> >
>> >
>> > Thanks for raising this Alex,
>> >
>> > I suggest doing a 1.7.2 patch release since we don't want to leave the
>> 1.7.x version in a broken state for the ADLSFileIO.
>> >
>> > Kind regards,
>> > Fokko
>> >
>> > Op di 17 dec 2024 om 07:40 schreef Jean-Baptiste Onofré <
>> j...@nanthrax.net>:
>> >
>> >
>> > Hi Alex,
>> >
>> > It was exactly my concern (and question) when I did the review on the
>> PR.
>> >
>> > I agree it's breaking change and definitely not good in a micro/patch
>> release.
>> >
>> > As 1.7.0 is still working, I would propose to wait 1.8.0 if possible:
>> > I'm actually preparing a new thread with 1.8.0 proposal (in terms of
>> > timing and content like improved security/auth flow in REST).
>> >
>> > Would it work for you or are you calling for 1.7.2 ?
>> >
>> > Regards
>> > JB
>> >
>> > On Tue, Dec 17, 2024 at 1:06 AM Alex Reid <alex.james.r...@gmail.com>
>> wrote:
>> >
>> >
>> > Just a heads up, but I believe this PR (
>> https://github.com/apache/iceberg/pull/11504) that was added to the
>> 1.7.1 introduced a breaking change for anyone previously supporting
>> ADLSFileIO / credentials with sasTokens. I added some details in the PR
>> comments, but will also provide those details here as well since the PR is
>> closed.
>> >
>> > On initialization of ADLSFileIO, AzureProperties is created and builds
>> a map of account -> sasToken here when you create ADLSFileIO using
>> adls.sas-token. as the credential mechanism.
>> >
>> > Prior to this change, the account passed in here (which is used to
>> lookup the sasToken from the map mentioned above) was parsed to include .
>> dfs.core.windows.net so when generating the adls.sas-token. property to
>> pass into ADLSFileIO, you needed to include .dfs.core.windows.net as
>> part of the adls.sas-token. property name. After this change, we are
>> parsing the ADLSLocation account to NOT include adls.sas-token., so now the
>> sasToken lookup from the map doesn't find the sasToken. When someone
>> updates to 1.7.1, they will/would also need to update how they are
>> configuring / building the ADLSFileIO properties when using sasTokens.
>> >
>> >
>> > -alex
>> >
>> >
>>
>

Reply via email to