kevinjqliu commented on PR #15407: URL: https://github.com/apache/iceberg/pull/15407#issuecomment-6407114404
Thanks for the quick turnaround @vladislav-sidorovich! Removing the golden files from the repo is great, but i don't think downloading them at test time is the right approach either. A few concerns: - if the download fails, the tests are silently skipped, so CI can go green without actually running them - the build now depends on a GitHub release we don't control. Release verification (`./gradlew build` from the source tarball) shouldn't need network access to a third-party repo - `delta-incubator/dat` doesn't seem to have a license - adds a new gradle plugin, writes to `~/.m2` under made-up coordinates, and adds `mavenLocal()` as a dependency source - nothing to do with the download, but `jackson-databind` is added as an `implementation` dependency and i don't see it used anywhere. is that needed? Can we generate the tables at runtime instead? The tests only need 2 tables: a basic one can be created with Delta Kernel in the unit tests, and the DV one with `delta-spark` in the integration tests (`testConversionWithDeletionVectors` already does something similar). That way everything is self-contained and we don't need to vendor or download anything. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
