Skip to content

Iceberg refactor into a separate lakehouse connector#1249

Open
nmahadevuni wants to merge 3 commits intoIBM:oss-mainfrom
nmahadevuni:iceberg-refactor-naveen-oss-main
Open

Iceberg refactor into a separate lakehouse connector#1249
nmahadevuni wants to merge 3 commits intoIBM:oss-mainfrom
nmahadevuni:iceberg-refactor-naveen-oss-main

Conversation

@nmahadevuni
Copy link
Copy Markdown
Member

Moving all required classes for Iceberg into a new lakehouse connector removing dependencies on hive connector.

Comment thread CMakeLists.txt
option(VELOX_ENABLE_AGGREGATES "Build aggregates." ON)
option(VELOX_ENABLE_HIVE_CONNECTOR "Build Hive connector." ON)
option(VELOX_ENABLE_HIVE_CONNECTOR "Build the Hive connector." ON)
#option(VELOX_ENABLE_HIVE_NEW_CONNECTOR "Build the new Hive connector." ON)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the unused lines

# See the License for the specific language governing permissions and
# limitations under the License.

#if(${VELOX_BUILD_TESTING})
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the tests run? If yes we need to uncomment this

@@ -16,9 +16,7 @@

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file names should not be ConnectorXXX any more. They need to be called IcebergXXX

@@ -16,9 +16,7 @@

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's already in the final leaf folder, there should not be XXXBase classes or files anymore. These Base classes shall be merged with the final classes, e.g. ConnectorConfigBase shall be merged into IcebergConfig

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants