I'm in for RC1, -1 Vote for RC0
On Wed, Jul 17, 2024 at 3:13 PM Jean-Baptiste Onofré <j...@nanthrax.net> wrote: > Hi Amogh > > Thanks ! Imho, I would prefer to change/"fix" the > TableMetadata.Builder constructor in 1.6.0. If we release like this, > It will be painful to deprecate and probably a bit confusing. > I think it's better to cancel RC0 and cut RC1 including visibility > change on the constructor, in order to have a "clean" 1.6.0 release. > > If there are no objections, I will cancel RC0 to prepare a RC1. > > Regards > JB > > On Wed, Jul 17, 2024 at 10:04 PM Amogh Jahagirdar <2am...@gmail.com> > wrote: > > > > Hey JB, > > > > Yes, I'd still hold on to my -1 (non-binding) vote due to the public > TableMetadata.Builder constructor which should be private. I have a PR > https://github.com/apache/iceberg/pull/10714 for addressing it (this > would need to be cherry picked as well on to the 1.6 branch). If folks are > in agreement with that, I'd recommend another candidate. If not (which I > can understand since maybe it's a bit overkill), we could just go through a > deprecation cycle. > > > > Thanks, > > > > Amogh Jahagirdar > > > > On Wed, Jul 17, 2024 at 1:39 PM Jean-Baptiste Onofré <j...@nanthrax.net> > wrote: > >> > >> Hi Amogh > >> > >> Are you keeping your -1 vote ? I'm a bit lost between your two messages > :) > >> > >> Thanks ! > >> Regards > >> JB > >> > >> On Wed, Jul 17, 2024 at 7:03 PM 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 >
