kbendick commented on code in PR #4553: URL: https://github.com/apache/iceberg/pull/4553#discussion_r859311698
########## flink/v1.15/flink/src/main/java/org/apache/iceberg/flink/FlinkCatalog.java: ########## @@ -0,0 +1,743 @@ +/* + * 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. + */ + +package org.apache.iceberg.flink; + +import java.io.Closeable; +import java.io.IOException; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import org.apache.flink.table.api.TableSchema; Review Comment: For this PR, we are migrating the 1.14 code with as few changes as possible to make it work with 1.15. so **TLDR** - In this PR, no, we will be keeping the old classes in an effort to move the code to support Flink 1.15 with as few changes as possible. That said, a small tangent as I personally am interested in seeing us move off of the deprecated APIs. There is interest in replacing TableSchema with the Schema and ResolvedSchema classes. I think there’s an open PR from @hililiwei on it. There’s also an issue I filed for looking into it. One thing that is missing from the conversation (and would be good to add to the PR from @hililiwei or to the issue if you’re interested in contributing or helping move the discussion forward) is a summary on the differences between the two and any possible implications that might have for us. Particularly for stake holders that use Iceberg a lot, and work on core Iceberg, but maybe only use Flink + Iceberg on occasion, it would help to have some sort of documentation / links / summary of the differences between the two schema classes so that core contributors will know what to look for when reviewing. If you’re interested in helping “modernize” some of the deprecated Flink API usage, I’d be happy to help point you to some PRs as well as give input that I’ve heard from others (or have myself) that would help move the process forward. But again, for this PR, the goal is simply to move the existing code with as few changes as possible to Flink 1.15. From there, we can make more changes, knowing that we’re starting from a “working” standpoint 👍 -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
