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

[FLINK-36245][FOLLOWUP] Remove the @Deprecated for ArrowSourceFunction and SinkFunction #26297

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
* @deprecated This class is based on the {@link SourceFunction} API, which is due to be removed.
* Use the new {@link org.apache.flink.api.connector.source.Source} API instead.
*/
@Deprecated
@Internal
public class ArrowSourceFunction extends RichParallelSourceFunction<RowData>
implements ResultTypeQueryable<RowData>, CheckpointedFunction {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,11 @@ protected DataStream<T> setConnectionType(StreamPartitioner<T> partitioner) {
*
* @param sinkFunction The object containing the sink's invoke function.
* @return The closed DataStream.
* @deprecated This method relies on the {@link SinkFunction} API, which is due to be removed.
Copy link
Contributor

@davidradl davidradl Mar 20, 2025

Choose a reason for hiding this comment

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

I see text "due to be removed." It will not be removed until at least Flink v3.
Idoes not look right to have a deprecated method referencing parameters are annotated with internal

Copy link
Contributor Author

@beliefer beliefer Mar 21, 2025

Choose a reason for hiding this comment

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

Yes. Because the SinkFunction will be removed in future versions, this function depends it.

ApiAnnotationRules lead to the error https://github.com/apache/flink/pull/26297/files#r1996509218
It means we should add @Deprecated

* Use the {@link #sinkTo(Sink)} method based on the new {@link
* org.apache.flink.api.connector.sink2.Sink} API instead.
*/
@Deprecated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To fix the issue.

2025-03-14T14:31:28.5664450Z Mar 14 14:31:28 14:31:28.565 [ERROR] Failures: 
2025-03-14T14:31:28.5666940Z Mar 14 14:31:28 14:31:28.565 [ERROR]   Architecture Violation [Priority: MEDIUM] - Rule 'Return and argument types of methods annotated with @Public must be annotated with @Public.' was violated (2 times):
2025-03-14T14:31:28.5669896Z Mar 14 14:31:28 org.apache.flink.streaming.api.datastream.DataStream.addSink(org.apache.flink.streaming.api.functions.sink.legacy.SinkFunction): Argument leaf type org.apache.flink.streaming.api.functions.sink.legacy.SinkFunction does not satisfy: reside outside of package 'org.apache.flink..' or reside in any package ['..shaded..'] or annotated with @Public or annotated with @Deprecated
2025-03-14T14:31:28.5673341Z Mar 14 14:31:28 org.apache.flink.streaming.api.datastream.KeyedStream.addSink(org.apache.flink.streaming.api.functions.sink.legacy.SinkFunction): Argument leaf type org.apache.flink.streaming.api.functions.sink.legacy.SinkFunction does not satisfy: reside outside of package 'org.apache.flink..' or reside in any package ['..shaded..'] or annotated with @Public or annotated with @Deprecated

public DataStreamSink<T> addSink(SinkFunction<T> sinkFunction) {

// read the output type of the input Transform to coax out errors about MissingTypeInfo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ protected <R> SingleOutputStreamOperator<R> doTransform(
}

@Override
@Deprecated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To fix the issue.

2025-03-15T03:40:51.6629822Z Mar 15 03:40:51 03:40:51.661 [ERROR] Tests run: 4, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 14.56 s <<< FAILURE! -- in org.apache.flink.architecture.rules.ApiAnnotationRules
2025-03-15T03:40:51.6631130Z Mar 15 03:40:51 03:40:51.662 [ERROR] ApiAnnotationRules.PUBLIC_API_METHODS_USE_ONLY_PUBLIC_API_TYPES -- Time elapsed: 0.118 s <<< FAILURE!
2025-03-15T03:40:51.6631774Z Mar 15 03:40:51 java.lang.AssertionError: 
2025-03-15T03:40:51.6632704Z Mar 15 03:40:51 Architecture Violation [Priority: MEDIUM] - Rule 'Return and argument types of methods annotated with @Public must be annotated with @Public.' was violated (1 times):
2025-03-15T03:40:51.6634529Z Mar 15 03:40:51 org.apache.flink.streaming.api.datastream.KeyedStream.addSink(org.apache.flink.streaming.api.functions.sink.legacy.SinkFunction): Argument leaf type org.apache.flink.streaming.api.functions.sink.legacy.SinkFunction does not satisfy: reside outside of package 'org.apache.flink..' or reside in any package ['..shaded..'] or annotated with @Public or annotated with @Deprecated

public DataStreamSink<T> addSink(SinkFunction<T> sinkFunction) {
DataStreamSink<T> result = super.addSink(sinkFunction);
result.getLegacyTransformation().setStateKeySelector(keySelector);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
* org.apache.flink.api.connector.sink2.Sink} interface instead.
*/
@Internal
@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we changing this tag?
It is was part of the legacy deprecated API - should this not be deprecated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should still be deprecated. I remove it since the annotation @deprecated duplicate with the comments @deprecated. And I follow up the #25331

public interface SinkFunction<IN> extends Function, Serializable {

/**
Expand Down