+1 (binding) I verified sigs/sums/build/test + ran some manual verification as well.
All looks good. -Dan On Wed, Jul 17, 2024 at 10:17 AM Carl Steinbach <c...@apache.org> wrote: > +1 (binding) > > > On Wed, Jul 17, 2024 at 10:05 AM Amogh Jahagirdar <2am...@gmail.com> > wrote: > >> Following up, >> >> I think I confused myself on the original issue >> https://github.com/apache/iceberg/issues/8756 when testing. That issue >> was specific to REST implementations which use `CatalogHandlers` like >> `RESTCatalogAdapter` used in our unit tests. The fix in #10369 does address >> that case for creation. When testing I was creating a v2 table and >> attempting to replace it with a v1 table which I think makes sense to fail >> because the downgrade would possibly be lossy, and then rolling back would >> not be safe. My original statement that "I think clients should not fail to >> build the change set with the format version change." is probably not >> correct for the downgrade case; it sounds best to fail on the client side >> since it's known to be unsafe. >> >> So from a fix/issue perspective, I think we're covered. However, in terms >> of APIs there's still the case of the public constructor that I added in >> #10369. That should not be public. >> >> Thanks and sorry for the confusion there, >> >> Amogh Jahagirdar >> >> >> >> >> On Wed, Jul 17, 2024 at 9:48 AM Amogh Jahagirdar <2am...@gmail.com> >> wrote: >> >>> I'm -1 (non-binding). >>> >>> Aside from running through the standard checks, I was testing >>> https://github.com/apache/iceberg/pull/10369/files via Spark against a >>> REST catalog (a non-testing REST catalog) and the issue still exists >>> although the stack trace just looks a bit different now. The fix currently >>> handles it on the catalog handler's side which really masks the real issue >>> of failing to build the changes for the replace on the client side (so imo >>> it's not really a fix looking back on it). I'm still thinking through what >>> a robust solution is; in the end for REST, the service needs to be able to >>> handle it, but I think clients should not fail to build the change set with >>> the format version change. >>> >>> To be clear, I don't think I'd block on a fix for this since I'm not >>> sure how common of a case it is for downgrade of version for a replace is >>> and if there's interest in a 1.6.1, we can aim for a more thought through >>> solution for that release. >>> >>> However the main concern I have is when I was going through the fix, the >>> table metadata builder constructor I added as part of this >>> https://github.com/apache/iceberg/pull/10369/files#diff-c540a31e66b157a8f080433c82a29a070096d0e08c6578a0099153f1229bdb7aR913 >>> is marked public, which I think I'd prefer to change to private upfront >>> rather than have to go through a deprecation cycle/revAPI changes. >>> >>> Thanks, >>> >>> Amogh Jahagirdar >>> >>> >>> On Wed, Jul 17, 2024 at 2:29 AM Honah J. <hon...@apache.org> wrote: >>> >>>> +1 (non-binding) >>>> >>>> - verified signature and checksum >>>> - verified license doc >>>> - verified build and tests with JDK 17 >>>> >>>> Best regards, >>>> Honah >>>> >>>> On Tue, Jul 16, 2024 at 10:40 PM Ajantha Bhat <ajanthab...@gmail.com> >>>> wrote: >>>> >>>>> Gentle reminder for the PMC members, we need at least two additional >>>>>> binding votes. >>>>> >>>>> >>>>> One additional vote. We have binding votes from Russell and Fokko >>>>> already. >>>>> >>>>> On Wed, Jul 17, 2024 at 10:54 AM Jean-Baptiste Onofré <j...@nanthrax.net> >>>>> wrote: >>>>> >>>>>> Gentle reminder for the PMC members, we need at least two additional >>>>>> binding votes. >>>>>> >>>>>> Thanks ! >>>>>> Regards >>>>>> JB >>>>>> >>>>>> On Fri, Jul 12, 2024 at 4:48 PM Jean-Baptiste Onofré <j...@nanthrax.net> >>>>>> wrote: >>>>>> > >>>>>> > Hi everyone, >>>>>> > >>>>>> > I propose that we release the following RC as the official Apache >>>>>> > Iceberg 1.6.0 release. >>>>>> > >>>>>> > The commit ID is ed228f79cd3e569e04af8a8ab411811803bf3a29 >>>>>> > * This corresponds to the tag: apache-iceberg-1.6.0-rc0 >>>>>> > * >>>>>> https://github.com/apache/iceberg/commits/apache-iceberg-1.6.0-rc0 >>>>>> > * >>>>>> https://github.com/apache/iceberg/tree/ed228f79cd3e569e04af8a8ab411811803bf3a29 >>>>>> > >>>>>> > The release tarball, signature, and checksums are here: >>>>>> > * >>>>>> https://dist.apache.org/repos/dist/dev/iceberg/apache-iceberg-1.6.0-rc0 >>>>>> > >>>>>> > You can find the KEYS file here: >>>>>> > * https://dist.apache.org/repos/dist/dev/iceberg/KEYS >>>>>> > >>>>>> > Convenience binary artifacts are staged on Nexus. The Maven >>>>>> repository URL is: >>>>>> > * >>>>>> https://repository.apache.org/content/repositories/orgapacheiceberg-1164/ >>>>>> > >>>>>> > Please download, verify, and test. >>>>>> > >>>>>> > Please vote in the next 72 hours. >>>>>> > >>>>>> > [ ] +1 Release this as Apache Iceberg 1.6.0 >>>>>> > [ ] +0 >>>>>> > [ ] -1 Do not release this because... >>>>>> > >>>>>> > Only PMC members have binding votes, but other community members are >>>>>> > encouraged to cast non-binding votes. This vote will pass if there >>>>>> are >>>>>> > 3 binding +1 votes and more binding +1 votes than -1 votes. >>>>>> > >>>>>> > Thanks, >>>>>> > Regards >>>>>> > JB >>>>>> >>>>>
