Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build warnings #370

Closed
fpj opened this issue May 7, 2020 · 1 comment · Fixed by #430
Closed

Fix build warnings #370

fpj opened this issue May 7, 2020 · 1 comment · Fixed by #430

Comments

@fpj
Copy link
Contributor

fpj commented May 7, 2020

Problem description
There are a number of warnings that need to be fixed, I'm posting them in a txt file attached.

Problem location
Build warnings.

Suggestions for an improvement
Fix the code to eliminate the warnings.

flink-connector-warnings.txt

@crazyzhou
Copy link
Contributor

crazyzhou commented May 8, 2020

There is an issue #237 for this, I also pay attention to this along the implementation. I have already taken effort to eliminate some in the 1.10 upgrade and more (mainly in Table API methods) is done in this PR #363 , but there are still one thing we can't easily upgrade that is the BatchTableSource and BatchTableSink. Here are some reasons.

  1. These APIs are deprecated because the Blink planner is unifying the Batch into the Stream. The suggested change is to use InputFormatTableSource and OutputFormatTableSink which is an extention of StreamTableSource/Sink and still marked experimental.
  2. I cannot directly change to use the new API, because we rely on the batch table factory https://github.com/pravega/flink-connectors/blob/master/src/main/java/io/pravega/connectors/flink/FlinkPravegaBatchTableSourceFactory.java#L38 which needs to give out a BatchTableSource/Sink. There is another issue Change to Blink planner implementation #356 for this change entirely.
  3. These APIs are still used in the Flink code. https://github.com/apache/flink/blob/master/flink-table/flink-table-api-java-bridge/src/main/java/org/apache/flink/table/sinks/CsvTableSink.java#L41
  4. As a bigger picture, I'm not going to rush on Change to Blink planner implementation #356 because the whole FLIP-32 is still in progress. And soon, it will have a new structure of the table API. (ref. FLIP-95). It will be better to wait the final interfaces ready on Flink side first.

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

Successfully merging a pull request may close this issue.

2 participants