umartin commented on PR #735: URL: https://github.com/apache/incubator-sedona/pull/735#issuecomment-1357488467
Nice work! Over all this looks good to me. Just a few notes: * Dependencies with compile scope are inherited. So python-adapter doesn't need to depend on sql, core and common. Only sql is needed. Same goes for the other modules. Maybe there is a reason why you had to repeat the dependencies that I'm missing. If so, ignore this comment :) * I think that separate shaded modules is a good idea. It would be nice to be able to opt out of shaded modules if you want to. In python and R projects you can just add the --packages flag to spark-submit or set the configuration property spark.jars.packages. Then spark will download any artifacts and their dependencies for you. Shading is never needed in Spark, regardless of which language you use, but it might be convenient depending on how you deploy your jobs. See https://spark.apache.org/docs/latest/submitting-applications.html#advanced-dependency-management If you're not that familiar with maven, the command `mvn dependency:tree` is a nice way to verify transitive dependencies. Verifying the shaded artifacts is harder. I would probably run `jar -tf sedona-spark-shaded.jar | sort` before and after this patch and diff the output. @jiayuasu publishing snapshots sounds lika a good idea -- 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]
