Hi, Ryan:
> - The REST catalog was unable to resolve icebergdata.minio causing 2 > test failures. I had to switch over to local FS to run tests or else > rest_catalog_test cases test_create_table and test_update_table would fail. > I suspect this is a docker problem because there is a link in the > docker-compose.yaml file to provide that alias > > Sorry, I don't know how you run the tests, but the integration tests are run in ci. So I think you are right, there are some problems with docker setup. > - The LICENSE file doesn’t contain any third-party code documentation. > That’s fine if there isn’t any copied code in the whole project, but seems > a little suspicious. Copying code is fairly common. Please help us make > sure code taken from other places is properly documented! > > As far as I know, currently we don't have codes directly copied from other projects. We have borrowed some codes from icelake <https://github.com/icelake-io/icelake> and JanKaul's iceberg-rust <https://github.com/JanKaul/iceberg-rust> , but we are also authors of those codes, so I think it's fine without documentation for these codes? > - The release script creates the tarball with git archive — which is > good — but doesn’t specify the specific files to include so you get > everything, including .gitignore, .github, .asf.yaml, and others that > aren't needed. I prefer being explicit about what is included to minimize > unnecessary files. > > Thanks for reporting this, we will improve it later. make check failed in a cargo sort command. It looks like this is not > intended to work in a release tarball? > After some investigation, I think this is a bug in `cargo sort`, the root cause is this line <https://github.com/DevinR528/cargo-sort/blob/55ec89082466f6bb246d870a8d56d166a8e1f08b/src/main.rs#L56> , e.g. since the path(/home/blue/tmp/apache-iceberg-rust-0.2.0-src) contains '.', it thinks it's a file without checking, and then it tries to read it as a file, and failed. A simple workaround is to rename apache-iceberg-rust-0.2.0-src to sth without dots, and it works. I'll report a bug for cargo sort. On Tue, Feb 20, 2024 at 8:54 AM Ryan Blue <b...@tabular.io> wrote: > +1 (binding) > > - Checked signature and checksum > - Ran the license check using docker run -it --rm -v > $(pwd):/github/workspace apache/skywalking-eyes header check > (I found this in the release.sh script) > - Verified .licenserc.yaml, LICENSE, and NOTICE > - Spot checked occurrences of ‘[Ff]rom’, ‘http’, and ‘[Cc]opied’ in > source to check for undocumented copied code (none) > - Compiled and tested in 1.75.0 using make test > - Built with cargo build --release > - Ran several makefile checks > > Non-blocking issues: > > - The REST catalog was unable to resolve icebergdata.minio causing 2 > test failures. I had to switch over to local FS to run tests or else > rest_catalog_test cases test_create_table and test_update_table would fail. > I suspect this is a docker problem because there is a link in the > docker-compose.yaml file to provide that alias > - The LICENSE file doesn’t contain any third-party code documentation. > That’s fine if there isn’t any copied code in the whole project, but seems > a little suspicious. Copying code is fairly common. Please help us make > sure code taken from other places is properly documented! > - The release script creates the tarball with git archive — which is > good — but doesn’t specify the specific files to include so you get > everything, including .gitignore, .github, .asf.yaml, and others that > aren't needed. I prefer being explicit about what is included to minimize > unnecessary files. > - make check failed in a cargo sort command. It looks like this is not > intended to work in a release tarball? > > cargo sort -c -w > error: no file found at: /home/blue/tmp/apache-iceberg-rust-0.2.0-src > make: *** [Makefile:33: cargo-sort] Error 1 > > > > On Mon, Feb 19, 2024 at 11:00 AM Jack Ye <yezhao...@gmail.com> wrote: > >> +1 (binding) >> >> Verified checksum, signature, license, note, ASF header >> Ran build and test >> Checked no unexpected binary files >> >> Best, >> Jack Ye >> >> On Mon, Feb 19, 2024 at 2:33 AM Jean-Baptiste Onofré <j...@nanthrax.net> >> wrote: >> >>> +1 (non binding) >>> >>> I checked: >>> - checksum and signature are correct >>> - ASF headers are there (not in the tsv files but not a problem) >>> - no binary found in the source distribution >>> >>> Good improvement for next releases: update NOTICE file to mention non >>> ASF dependencies (listed in DEPENDENCIES.rust.tsv) with summary of the >>> licenses. I will propose a PR about that. >>> >>> Thanks ! >>> Regards >>> JB >>> >>> On Thu, Feb 15, 2024 at 1:52 PM Fokko Driesprong <fo...@apache.org> >>> wrote: >>> > >>> > Hello, Apache Iceberg Rust Community, >>> > >>> > This is a call for a vote to release Apache Iceberg Rust version 0.2.0. >>> > >>> > The tag to be voted on is 0.2.0-rc.1. >>> > >>> > This first release provides integration with the REST catalog and a >>> lot of scaffolding that's needed for reading the data. >>> > >>> > The release candidate: >>> > >>> > >>> https://dist.apache.org/repos/dist/dev/iceberg/iceberg-rust-0.2.0-rc.1/ >>> > >>> > Keys to verify the release candidate: >>> > >>> > https://downloads.apache.org/iceberg/KEYS >>> > >>> > Git tag for the release: >>> > >>> > https://github.com/apache/iceberg-rust/releases/tag/v0.2.0-rc.1 >>> > >>> > Please download, verify, and test. >>> > >>> > The VOTE will be open for at least 72 hours and until the necessary >>> > number of votes are reached. >>> > >>> > [ ] +1 approve >>> > [ ] +0 no opinion >>> > [ ] -1 disapprove with the reason >>> > >>> > To learn more about Apache Iceberg, please see >>> https://rust.iceberg.apache.org/ >>> > >>> > Checklist for reference: >>> > >>> > [ ] Download links are valid. >>> > [ ] Checksums and signatures. >>> > [ ] LICENSE/NOTICE files exist >>> > [ ] No unexpected binary files >>> > [ ] All source files have ASF headers >>> > [ ] Can compile from source >>> > >>> > More detailed checklist please refer to: >>> > https://github.com/apache/iceberg-rust/tree/main/scripts >>> > >>> > To compile from the source, please refer to: >>> > https://github.com/apache/iceberg-rust/blob/main/CONTRIBUTING.md >>> > >>> > Huge thanks to: Amogh Jahagirdar, Chengxu Bian, Christian Daudt, >>> Farooq Qaiser, JanKaul, Manu Zhang, Mark Grey, Renjie Liu, Tyler Schauer, >>> Xiaoyang Liu, Xuanwo, ZENOTME, barronw, hiirrxnn, y0psolo, yi wang, zhjwpku >>> and of course dependabot[bot] for working on this first release! >>> > >>> > Here is a Python script in release to help you verify the release >>> candidate: >>> > >>> > ./scripts/verify.py >>> > >>> > Please consider this my +1 (binding) vote. I've ran the license checks >>> and tested against the REST catalog, and it worked like a charm. Code can >>> be found here: >>> https://github.com/Fokko/hello-iceberg/blob/master/src/main.rs >>> > >>> > Thanks, Fokko >>> >> > > -- > Ryan Blue > Tabular >
