yidawang-shopify commented on code in PR #17668:
URL: https://github.com/apache/iceberg/pull/17668#discussion_r4084709804
##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/sink/IcebergSink.java:
##########
@@ -749,13 +782,16 @@ public Builder toBranch(String branch) {
return this;
}
- IcebergSink build() {
-
- Preconditions.checkArgument(
- inputCreator != null,
- "Please use forRowData() or forMapperOutputType() to initialize the
input DataStream.");
+ /**
+ * Builds the sink without wiring it into a {@link DataStream}. Use this
when Flink calls {@code
+ * DataStream#sinkTo} itself, as it does for a {@code SinkV2Provider}; use
{@link #append()} to
+ * attach the sink to an input stream directly.
+ */
+ public IcebergSink build() {
Review Comment:
For this PR, I think it might be best to leave this one public.
This build method lives in `IcebergSink`:
```java
package org.apache.iceberg.flink.sink;
```
When we actually use the sink, the sink is called from an `IcebergTableSink`:
```java
package org.apache.iceberg.flink; // This is a different package from the
IcebergSink
...
import org.apache.iceberg.flink.sink.IcebergSink; // Here we import the
IcebergSink to call it's builder
...
// The actual call:
private IcebergSink buildIcebergSink() {
ResolvedSchema physicalColumnsOnlySchema = physicalColumnsOnlySchema();
return icebergSinkBuilder(
IcebergSink.builder(),
equalityColumns(physicalColumnsOnlySchema),
physicalColumnsOnlySchema)
.build();
}
```
The right approach will be align these two packages? Move `IcebergTableSink`
into `org.apache.iceberg.flink.sink`. This is the convention set in
`IcebergTableSource` and `IcebergSource`, these two both live in `package
org.apache.iceberg.flink.source;` instead two seperated one.
Let me know what you think about this.
--
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]
