[lake] Fix iceberg aws glue catalog class not found. #2038
Conversation
| <excludes> | ||
| <exclude>org/apache/iceberg/hive/**</exclude> | ||
| <exclude>org/apache/iceberg/aws/**</exclude> | ||
| <exclude>org/apache/iceberg/nessie/**</exclude> | ||
| </excludes> |
There was a problem hiding this comment.
Will this approach break the intended isolation of dependencies? E.g. if something else provide iceberg hive/aws/nessie classes in the classpath of version different to what Fluss-lake-icebery depends on, I'd expect that we'll see issues such as NoSuchMethodError.
Should the approach instead be fixing the shading configuration so that fluss.lake.iceberg contains shaded hive/aws/nessie packages?
There was a problem hiding this comment.
It is not recommended that fluss.lake.iceberg contains the shadow iceberg plug-in package. It will cause the fluss.lake.iceberg package to be bloated and cause some problems with third-party plug-ins.
There was a problem hiding this comment.
Pull request overview
This PR fixes a class not found issue for the Iceberg AWS Glue catalog by excluding certain Iceberg packages from shading. The problem occurred because the AWS Glue catalog classes were being relocated during the shade plugin process, making them unavailable at runtime.
Key Changes:
- Modified Maven shade plugin configuration to exclude
org/apache/iceberg/hive/**,org/apache/iceberg/aws/**, andorg/apache/iceberg/nessie/**from relocation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <relocations> | ||
| <!-- Shade Iceberg to Fluss namespace for complete isolation --> | ||
| <relocation> | ||
| <pattern>org.apache.iceberg</pattern> |
There was a problem hiding this comment.
I'm thinking of remove the shadeing..We introduce the shading is to resolve the class conflict in fluss-quick-start image which contains paimon. But in #1997 , we decide to remove fluss-quick-start which mix iceberg and paimon. We will run container with only iceberg, no paimon. In this case, I think we can remove the shadeing.
@MehulBatra WDYT?
There was a problem hiding this comment.
yes this seems a good idea @luoyuxia if we are moving with standard #1997 approach, I would suggest @zhuangchong please test the setup after making changes, if the quickstart working as expected!
There was a problem hiding this comment.
Great call! If the dependency isolation is not required anymore, removing shading is much better than leaving some transitives unshaded.
There was a problem hiding this comment.
+1 on removing the shading of Iceberg classes
zhuangchong
force-pushed
the
lake-iceberg-aws
branch
from
4efbdf6 to
24db6cc
Compare
5 participants
Purpose
Linked issue: close #2013
Before building the package jar:
After building the package jar:
Brief change log
Tests
API and Format
Documentation