Thanks Marc. I gonna review the new PR.
Regards JB Le ven. 20 déc. 2024 à 00:48, Marc Cenac <marc.ce...@datadoghq.com.invalid> a écrit : > 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 >>> > >>> > >>> >>
