vinishjail97 commented on code in PR #612: URL: https://github.com/apache/incubator-xtable/pull/612#discussion_r3798227945
########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure Review Comment: I think RFC-2 may already be taken? `rfc/rfc-2/2 - Deletion Info Conversion.md` landed in dc209962, after you opened this. Would you mind renumbering, and does the `rfc/rfc-<n>/rfc-<n>.md` layout work here too? ########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure + +## Proposers + +- @the-other-tim-brown + +## Approvers +- @<approver1 github username> +- @<approver2 github username> + +## Status + +GH Feature Request: https://github.com/apache/incubator-xtable/issues/611 + +> Please keep the status updated in `rfc/README.md`. + +## Abstract +This proposal aims to make it easier for users to run the XTable code when incorporating it into their own project or leveraging the pre-built jars published by the XTable project. +As part of this, there is a secondary goal to make sure that the jars published, including the bundled jars, are trimmed down to the minimum size possible to prevent bloat for our users and to make sure there are no compliance issues with the dependencies. + +## Background +Currently, XTable users have two options for running the conversion code. +They can create their own jar with the XTable dependencies specified in their build file or they can generate the xtable-utilities bundled jar and run that on its own. +There are difficulties when using either approach currently. + +When running the prebuilt utilities bundle jar, the user will bring in all the required dependencies which currently include three table formats, spark, and hadoop. +This results in a very large jar containing more than the user really needs. + +When building your own jar, you can run into issues with conflicts in versions for the specific table formats you want to use since the user will often already have an implementation of at least one format on their classpath. +These conflicts may only surface as runtime errors when there is difference in method signature for example. +This can make the experience brittle for the end user by requiring them to closely follow any upgrades within the XTable repo as well. +In this scenario the user will also have dependencies on all 3 table formats by default. + +## Implementation +In order to allow users to include only the dependencies that they need for their use case, the repo will need to have modules per table format and per catalog client in the future. +This will allow the user to include only the relevant modules at runtime to fit their needs. Furthermore, each of these sub-modules will publish a shaded jar that shades the format specific dependencies. Review Comment: Does "shades" here mean relocating packages, or just bundling them? I ask because `xtable-utilities/pom.xml` today has 480 `<include>` entries and no `<relocation>`, so I am not sure the existing bundle avoids the conflicts described in the Background. If we do relocate, how would we handle the lookups that go through class-name strings, such as the payload class in `hoodie.properties` or Iceberg's `catalog-impl`? Would `ServicesResourceTransformer` cover the `META-INF/services` side? ########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure + +## Proposers + +- @the-other-tim-brown + +## Approvers +- @<approver1 github username> +- @<approver2 github username> + +## Status + +GH Feature Request: https://github.com/apache/incubator-xtable/issues/611 + +> Please keep the status updated in `rfc/README.md`. + +## Abstract +This proposal aims to make it easier for users to run the XTable code when incorporating it into their own project or leveraging the pre-built jars published by the XTable project. +As part of this, there is a secondary goal to make sure that the jars published, including the bundled jars, are trimmed down to the minimum size possible to prevent bloat for our users and to make sure there are no compliance issues with the dependencies. + +## Background +Currently, XTable users have two options for running the conversion code. +They can create their own jar with the XTable dependencies specified in their build file or they can generate the xtable-utilities bundled jar and run that on its own. +There are difficulties when using either approach currently. + +When running the prebuilt utilities bundle jar, the user will bring in all the required dependencies which currently include three table formats, spark, and hadoop. +This results in a very large jar containing more than the user really needs. + +When building your own jar, you can run into issues with conflicts in versions for the specific table formats you want to use since the user will often already have an implementation of at least one format on their classpath. +These conflicts may only surface as runtime errors when there is difference in method signature for example. +This can make the experience brittle for the end user by requiring them to closely follow any upgrades within the XTable repo as well. +In this scenario the user will also have dependencies on all 3 table formats by default. + +## Implementation +In order to allow users to include only the dependencies that they need for their use case, the repo will need to have modules per table format and per catalog client in the future. +This will allow the user to include only the relevant modules at runtime to fit their needs. Furthermore, each of these sub-modules will publish a shaded jar that shades the format specific dependencies. +This will allow the user to easily add jars and the required dependencies when executing the standalone RunSync command as well as avoid issues with version differences when adding these jars to an existing class-path that may contain table format related dependencies already. +In addition to this, the jars will still be published to allow users to compose their own shaded jar if they have specific needs or customizations that they want. Review Comment: Today all four targets sit in one `META-INF/services` file in `xtable-core`. After the split, what does a user see if they ask for `ICEBERG` without that module on the classpath? Would it be worth committing to an error that names the missing jar, given `ServiceLoader.hasNext()` can throw `ServiceConfigurationError`? ########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure + +## Proposers + +- @the-other-tim-brown + +## Approvers +- @<approver1 github username> +- @<approver2 github username> + +## Status + +GH Feature Request: https://github.com/apache/incubator-xtable/issues/611 + +> Please keep the status updated in `rfc/README.md`. + +## Abstract +This proposal aims to make it easier for users to run the XTable code when incorporating it into their own project or leveraging the pre-built jars published by the XTable project. +As part of this, there is a secondary goal to make sure that the jars published, including the bundled jars, are trimmed down to the minimum size possible to prevent bloat for our users and to make sure there are no compliance issues with the dependencies. + +## Background +Currently, XTable users have two options for running the conversion code. +They can create their own jar with the XTable dependencies specified in their build file or they can generate the xtable-utilities bundled jar and run that on its own. +There are difficulties when using either approach currently. + +When running the prebuilt utilities bundle jar, the user will bring in all the required dependencies which currently include three table formats, spark, and hadoop. Review Comment: Small correction since many changes have been added in the main branch: `xtable-core` carries six format paths rather than three (hudi, iceberg, delta, kernel, paimon, parquet). Since the format deps are already `provided`, is the real problem that core *classes* reference every format, so a wrong version fails at runtime? ########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure + +## Proposers + +- @the-other-tim-brown + +## Approvers +- @<approver1 github username> +- @<approver2 github username> + +## Status + +GH Feature Request: https://github.com/apache/incubator-xtable/issues/611 + +> Please keep the status updated in `rfc/README.md`. + +## Abstract +This proposal aims to make it easier for users to run the XTable code when incorporating it into their own project or leveraging the pre-built jars published by the XTable project. +As part of this, there is a secondary goal to make sure that the jars published, including the bundled jars, are trimmed down to the minimum size possible to prevent bloat for our users and to make sure there are no compliance issues with the dependencies. Review Comment: How do you see the split affecting the LICENSE and NOTICE work? Each published bundle needs its own, and we only just fixed for a single bundle in PR https://github.com/apache/incubator-xtable/pull/700 ########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure + +## Proposers + +- @the-other-tim-brown + +## Approvers +- @<approver1 github username> Review Comment: A few nits: the Approvers placeholders and Status are unfilled, `rfc/README.md` referenced on line 31 does not exist yet, and the image path on line 58 renders broken on GitHub (`rfc/rfc-1/rfc-1.md` has the same issue). Missing newline at EOF too. ########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure + +## Proposers + +- @the-other-tim-brown + +## Approvers +- @<approver1 github username> +- @<approver2 github username> + +## Status + +GH Feature Request: https://github.com/apache/incubator-xtable/issues/611 + +> Please keep the status updated in `rfc/README.md`. + +## Abstract +This proposal aims to make it easier for users to run the XTable code when incorporating it into their own project or leveraging the pre-built jars published by the XTable project. +As part of this, there is a secondary goal to make sure that the jars published, including the bundled jars, are trimmed down to the minimum size possible to prevent bloat for our users and to make sure there are no compliance issues with the dependencies. + +## Background +Currently, XTable users have two options for running the conversion code. +They can create their own jar with the XTable dependencies specified in their build file or they can generate the xtable-utilities bundled jar and run that on its own. +There are difficulties when using either approach currently. + +When running the prebuilt utilities bundle jar, the user will bring in all the required dependencies which currently include three table formats, spark, and hadoop. +This results in a very large jar containing more than the user really needs. + +When building your own jar, you can run into issues with conflicts in versions for the specific table formats you want to use since the user will often already have an implementation of at least one format on their classpath. +These conflicts may only surface as runtime errors when there is difference in method signature for example. +This can make the experience brittle for the end user by requiring them to closely follow any upgrades within the XTable repo as well. +In this scenario the user will also have dependencies on all 3 table formats by default. + +## Implementation +In order to allow users to include only the dependencies that they need for their use case, the repo will need to have modules per table format and per catalog client in the future. +This will allow the user to include only the relevant modules at runtime to fit their needs. Furthermore, each of these sub-modules will publish a shaded jar that shades the format specific dependencies. +This will allow the user to easily add jars and the required dependencies when executing the standalone RunSync command as well as avoid issues with version differences when adding these jars to an existing class-path that may contain table format related dependencies already. Review Comment: What does each format bundle include, and what stays `provided`? My worry is that if `xtable-api` is packaged (or relocated) per bundle, the `ConversionTarget` interface differs per bundle and `ServiceLoader` finds nothing. Is the intent that only third-party format deps are relocated? ########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure + +## Proposers + +- @the-other-tim-brown + +## Approvers +- @<approver1 github username> +- @<approver2 github username> + +## Status + +GH Feature Request: https://github.com/apache/incubator-xtable/issues/611 + +> Please keep the status updated in `rfc/README.md`. + +## Abstract +This proposal aims to make it easier for users to run the XTable code when incorporating it into their own project or leveraging the pre-built jars published by the XTable project. +As part of this, there is a secondary goal to make sure that the jars published, including the bundled jars, are trimmed down to the minimum size possible to prevent bloat for our users and to make sure there are no compliance issues with the dependencies. + +## Background +Currently, XTable users have two options for running the conversion code. +They can create their own jar with the XTable dependencies specified in their build file or they can generate the xtable-utilities bundled jar and run that on its own. +There are difficulties when using either approach currently. + +When running the prebuilt utilities bundle jar, the user will bring in all the required dependencies which currently include three table formats, spark, and hadoop. +This results in a very large jar containing more than the user really needs. + +When building your own jar, you can run into issues with conflicts in versions for the specific table formats you want to use since the user will often already have an implementation of at least one format on their classpath. +These conflicts may only surface as runtime errors when there is difference in method signature for example. +This can make the experience brittle for the end user by requiring them to closely follow any upgrades within the XTable repo as well. +In this scenario the user will also have dependencies on all 3 table formats by default. + +## Implementation +In order to allow users to include only the dependencies that they need for their use case, the repo will need to have modules per table format and per catalog client in the future. Review Comment: On @ashvina's question about sub-tasks, would it help to call out the core-to-format imports that need inverting first? The ones I noticed are `ConversionTargetFactory` importing `DeltaConversionTargetConfig` and `DeltaKernelConversionTarget`, and `CatalogUtils` importing `HudiCatalogPartitionSyncTool`. Could those be separate PRs ahead of the module moves? ########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure + +## Proposers + +- @the-other-tim-brown + +## Approvers +- @<approver1 github username> +- @<approver2 github username> + +## Status + +GH Feature Request: https://github.com/apache/incubator-xtable/issues/611 + +> Please keep the status updated in `rfc/README.md`. + +## Abstract +This proposal aims to make it easier for users to run the XTable code when incorporating it into their own project or leveraging the pre-built jars published by the XTable project. +As part of this, there is a secondary goal to make sure that the jars published, including the bundled jars, are trimmed down to the minimum size possible to prevent bloat for our users and to make sure there are no compliance issues with the dependencies. + +## Background +Currently, XTable users have two options for running the conversion code. +They can create their own jar with the XTable dependencies specified in their build file or they can generate the xtable-utilities bundled jar and run that on its own. +There are difficulties when using either approach currently. + +When running the prebuilt utilities bundle jar, the user will bring in all the required dependencies which currently include three table formats, spark, and hadoop. +This results in a very large jar containing more than the user really needs. + +When building your own jar, you can run into issues with conflicts in versions for the specific table formats you want to use since the user will often already have an implementation of at least one format on their classpath. +These conflicts may only surface as runtime errors when there is difference in method signature for example. +This can make the experience brittle for the end user by requiring them to closely follow any upgrades within the XTable repo as well. +In this scenario the user will also have dependencies on all 3 table formats by default. + +## Implementation +In order to allow users to include only the dependencies that they need for their use case, the repo will need to have modules per table format and per catalog client in the future. +This will allow the user to include only the relevant modules at runtime to fit their needs. Furthermore, each of these sub-modules will publish a shaded jar that shades the format specific dependencies. +This will allow the user to easily add jars and the required dependencies when executing the standalone RunSync command as well as avoid issues with version differences when adding these jars to an existing class-path that may contain table format related dependencies already. +In addition to this, the jars will still be published to allow users to compose their own shaded jar if they have specific needs or customizations that they want. + +We can also create these modules with a suffix indicating which version of the format is supported to allow for different implementations or versions of dependencies for the same format. For example, have an xtable-iceberg-v2 and xtable-iceberg-v3 module. + + Review Comment: Could the diagram be refreshed to include Paimon, Parquet, Kernel, `xtable-aws`, `xtable-hive-metastore` and `xtable-service`? Also, where do the two Delta paths land? `delta-core` pulls in Spark and Scala, Kernel does not, so does `xtable-delta` become one module or two? ########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure + +## Proposers + +- @the-other-tim-brown + +## Approvers +- @<approver1 github username> +- @<approver2 github username> + +## Status + +GH Feature Request: https://github.com/apache/incubator-xtable/issues/611 + +> Please keep the status updated in `rfc/README.md`. + +## Abstract +This proposal aims to make it easier for users to run the XTable code when incorporating it into their own project or leveraging the pre-built jars published by the XTable project. +As part of this, there is a secondary goal to make sure that the jars published, including the bundled jars, are trimmed down to the minimum size possible to prevent bloat for our users and to make sure there are no compliance issues with the dependencies. + +## Background +Currently, XTable users have two options for running the conversion code. +They can create their own jar with the XTable dependencies specified in their build file or they can generate the xtable-utilities bundled jar and run that on its own. +There are difficulties when using either approach currently. + +When running the prebuilt utilities bundle jar, the user will bring in all the required dependencies which currently include three table formats, spark, and hadoop. +This results in a very large jar containing more than the user really needs. + +When building your own jar, you can run into issues with conflicts in versions for the specific table formats you want to use since the user will often already have an implementation of at least one format on their classpath. +These conflicts may only surface as runtime errors when there is difference in method signature for example. +This can make the experience brittle for the end user by requiring them to closely follow any upgrades within the XTable repo as well. +In this scenario the user will also have dependencies on all 3 table formats by default. + +## Implementation +In order to allow users to include only the dependencies that they need for their use case, the repo will need to have modules per table format and per catalog client in the future. +This will allow the user to include only the relevant modules at runtime to fit their needs. Furthermore, each of these sub-modules will publish a shaded jar that shades the format specific dependencies. +This will allow the user to easily add jars and the required dependencies when executing the standalone RunSync command as well as avoid issues with version differences when adding these jars to an existing class-path that may contain table format related dependencies already. +In addition to this, the jars will still be published to allow users to compose their own shaded jar if they have specific needs or customizations that they want. + +We can also create these modules with a suffix indicating which version of the format is supported to allow for different implementations or versions of dependencies for the same format. For example, have an xtable-iceberg-v2 and xtable-iceberg-v3 module. Review Comment: Does the suffix mean the format spec version or the library version? I ask because iceberg-core 1.9.2 already writes both spec v2 and v3, whereas Hudi 0.x vs 1.x genuinely cannot share a module (hence `main-hudi-0x`). Would it be simpler to add the suffix only when two library versions must coexist? That might also answer @ashvina's question above. ########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure + +## Proposers + +- @the-other-tim-brown + +## Approvers +- @<approver1 github username> +- @<approver2 github username> + +## Status + +GH Feature Request: https://github.com/apache/incubator-xtable/issues/611 + +> Please keep the status updated in `rfc/README.md`. + +## Abstract +This proposal aims to make it easier for users to run the XTable code when incorporating it into their own project or leveraging the pre-built jars published by the XTable project. +As part of this, there is a secondary goal to make sure that the jars published, including the bundled jars, are trimmed down to the minimum size possible to prevent bloat for our users and to make sure there are no compliance issues with the dependencies. + +## Background +Currently, XTable users have two options for running the conversion code. +They can create their own jar with the XTable dependencies specified in their build file or they can generate the xtable-utilities bundled jar and run that on its own. +There are difficulties when using either approach currently. + +When running the prebuilt utilities bundle jar, the user will bring in all the required dependencies which currently include three table formats, spark, and hadoop. +This results in a very large jar containing more than the user really needs. + +When building your own jar, you can run into issues with conflicts in versions for the specific table formats you want to use since the user will often already have an implementation of at least one format on their classpath. +These conflicts may only surface as runtime errors when there is difference in method signature for example. +This can make the experience brittle for the end user by requiring them to closely follow any upgrades within the XTable repo as well. +In this scenario the user will also have dependencies on all 3 table formats by default. + +## Implementation +In order to allow users to include only the dependencies that they need for their use case, the repo will need to have modules per table format and per catalog client in the future. +This will allow the user to include only the relevant modules at runtime to fit their needs. Furthermore, each of these sub-modules will publish a shaded jar that shades the format specific dependencies. +This will allow the user to easily add jars and the required dependencies when executing the standalone RunSync command as well as avoid issues with version differences when adding these jars to an existing class-path that may contain table format related dependencies already. +In addition to this, the jars will still be published to allow users to compose their own shaded jar if they have specific needs or customizations that they want. + +We can also create these modules with a suffix indicating which version of the format is supported to allow for different implementations or versions of dependencies for the same format. For example, have an xtable-iceberg-v2 and xtable-iceberg-v3 module. + + + +## Rollout/Adoption Plan + +- Are there any breaking changes as part of this new feature/functionality? +Yes, users will need to add new jars to their class-paths when running commands. Additionally, users may need to import different modules into their builds if they are constructing their own jar. Review Comment: For users who upgrade in place and keep running the same bundle command, the failure would land at runtime rather than build time. Would it be worth publishing an all-formats bundle for a release or two, so `RunSync` keeps working and only users who want a small jar have to change? ########## rfc/rfc2.md: ########## @@ -0,0 +1,73 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +# RFC-2: Module Structure + +## Proposers + +- @the-other-tim-brown + +## Approvers +- @<approver1 github username> +- @<approver2 github username> + +## Status + +GH Feature Request: https://github.com/apache/incubator-xtable/issues/611 + +> Please keep the status updated in `rfc/README.md`. + +## Abstract +This proposal aims to make it easier for users to run the XTable code when incorporating it into their own project or leveraging the pre-built jars published by the XTable project. +As part of this, there is a secondary goal to make sure that the jars published, including the bundled jars, are trimmed down to the minimum size possible to prevent bloat for our users and to make sure there are no compliance issues with the dependencies. + +## Background +Currently, XTable users have two options for running the conversion code. +They can create their own jar with the XTable dependencies specified in their build file or they can generate the xtable-utilities bundled jar and run that on its own. +There are difficulties when using either approach currently. + +When running the prebuilt utilities bundle jar, the user will bring in all the required dependencies which currently include three table formats, spark, and hadoop. +This results in a very large jar containing more than the user really needs. + +When building your own jar, you can run into issues with conflicts in versions for the specific table formats you want to use since the user will often already have an implementation of at least one format on their classpath. +These conflicts may only surface as runtime errors when there is difference in method signature for example. +This can make the experience brittle for the end user by requiring them to closely follow any upgrades within the XTable repo as well. +In this scenario the user will also have dependencies on all 3 table formats by default. + +## Implementation +In order to allow users to include only the dependencies that they need for their use case, the repo will need to have modules per table format and per catalog client in the future. +This will allow the user to include only the relevant modules at runtime to fit their needs. Furthermore, each of these sub-modules will publish a shaded jar that shades the format specific dependencies. +This will allow the user to easily add jars and the required dependencies when executing the standalone RunSync command as well as avoid issues with version differences when adding these jars to an existing class-path that may contain table format related dependencies already. +In addition to this, the jars will still be published to allow users to compose their own shaded jar if they have specific needs or customizations that they want. + +We can also create these modules with a suffix indicating which version of the format is supported to allow for different implementations or versions of dependencies for the same format. For example, have an xtable-iceberg-v2 and xtable-iceberg-v3 module. + + + +## Rollout/Adoption Plan + +- Are there any breaking changes as part of this new feature/functionality? +Yes, users will need to add new jars to their class-paths when running commands. Additionally, users may need to import different modules into their builds if they are constructing their own jar. +- What impact (if any) will there be on existing users? +The user should be able to use only the dependencies for the formats that they are interested in converting between. +- If we are changing behavior how will we phase out the older behavior? When will we remove the existing behavior? +There will be upgrade instructions included in the next set of Release Notes +- If we need special migration tools, describe them here. +None needed, just need to update documentation. + +## Test Plan + +Describe in few sentences how the RFC will be tested. How will we know that the implementation works as expected? How will we know nothing breaks? Review Comment: Could we fill in the Test Plan? A few things I would find convincing: - `ITConversionController` green against the split modules (related to #886). - The bundle validation flow you mentioned on dev@, running each bundle alone. - A run with a conflicting version of that format library on the classpath, since that is what proves the relocation claim. - Jar sizes before and after, given the size goal in the abstract. Would it also be worth noting the merge order here, i.e. test module first, then one PR per format? -- 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]
