snowflake-ingest-java
snowflake-ingest-java copied to clipboard
SNOW-1257851 Move to using snowjdbc thin jar instead of the fat jar
https://github.com/snowflakedb/snowflake-ingest-java/issues/671, move from jdbc fat jar to thin jar.
JIRA: https://snowflakecomputing.atlassian.net/browse/SNOW-1257851 https://snowflakecomputing.atlassian.net/browse/SNOW-818172
@sfc-gh-lsembera @sfc-gh-alhuang I want to revisit our decision of NOT shading the JDBC dependency after this change, we have seen too many issues that customer is using an older version of JDBC with the latest SDK and something is not working due to bugs in the older versions of JDBC. Usually these issues are hard to debug as well since it requires JDBC logs and we don't know if the issues are in SDK or JDBC. I believe one of the main reason to not shading was due to the user jar size, which will be addressed in this PR, WDYT?
@sfc-gh-lsembera @sfc-gh-alhuang I want to revisit our decision of NOT shading the JDBC dependency after this change, we have seen too many issues that customer is using an older version of JDBC with the latest SDK and something is not working due to bugs in the older versions of JDBC. Usually these issues are hard to debug as well since it requires JDBC logs and we don't know if the issues are in SDK or JDBC. I believe one of the main reason to not shading was due to the user jar size, which will be addressed in this PR, WDYT?
Yes, this PR shades JDBC in the shaded JAR, the exclusion for snowflake-jdbc is gone, (see here).
May be this is already discussed, but can we confirm from jdbc team this is okay to use in production? https://docs.snowflake.com/en/release-notes/clients-drivers/jdbc-2024 release notes says, it is an experimental feature - not sure what it means -/ (PrPr?)
but if we have confirmed this is okay to use, please ignore my above comment.
May be this is already discussed, but can we confirm from jdbc team this is okay to use in production? https://docs.snowflake.com/en/release-notes/clients-drivers/jdbc-2024 release notes says, it is an experimental feature - not sure what it means -/ (PrPr?)
but if we have confirmed this is okay to use, please ignore my above comment.
I believe we did check with them, but I didn't see the note, it's always good to double check again :)
Thanks @sfc-gh-alhuang ! No concerns from me as long as @sfc-gh-lsembera signed off, could you wait for release 2.1.1 to go out before merging this change?
@sfc-gh-alhuang @sfc-gh-tzhang Are we going to merge this one? If so, we should do it early, so we can internally switch to the new SNAPSHOT version, so it gets thoroughly tested before the release.