search

snowflakedb / snowflake-ingest-java

Java SDK for the Snowflake Ingest Service -
http://www.snowflake.net
Apache License 2.0
67 stars 52 forks source link

SNOW-1257851 Move to using snowjdbc thin jar instead of the fat jar #733

Open sfc-gh-alhuang opened 5 months ago

sfc-gh-alhuang commented 5 months ago

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-tzhang commented 4 months ago

@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 commented 4 months ago

@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).

sfc-gh-japatel commented 4 months ago

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.

sfc-gh-tzhang commented 4 months ago

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 :)

sfc-gh-tzhang commented 4 months ago

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-lsembera commented 3 months ago

@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.