You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "Fokko (via GitHub)" <gi...@apache.org> on 2023/03/10 20:16:50 UTC
[GitHub] [iceberg] Fokko opened a new pull request, #7070: Flink: Update the docs
Fokko opened a new pull request, #7070:
URL: https://github.com/apache/iceberg/pull/7070
The docs were still mentioning Flink 1.11, and was long overdue for an overhaul. Updated the links and also did some rewriting of the docs by removing `we`.
The Python quickstart became much simpler since the jars now ship with the package itself: https://pypi.org/project/apache-flink-libraries/1.16.1/
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on a diff in pull request #7070: Flink: Update the docs
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#discussion_r1132942973
##########
docs/flink-getting-started.md:
##########
@@ -710,7 +687,7 @@ env.execute("Test Iceberg DataStream");
### Upsert data
-To upsert the data in existing iceberg table, we could set the `upsert` flag in FlinkSink builder. The table must use v2 table format and have a primary key.
+To upsert the data in existing iceberg table, set the `upsert` flag in FlinkSink builder. The table must use v2 table format and have a primary key.
Review Comment:
similarly reverse the order?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a diff in pull request #7070: Flink: Update the docs
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#discussion_r1133141480
##########
docs/flink-getting-started.md:
##########
@@ -687,11 +664,11 @@ FlinkSink.forRowData(input)
env.execute("Test Iceberg DataStream");
```
-The iceberg API also allows users to write generic `DataStream<T>` to iceberg table, more example could be found in this [unit test](https://github.com/apache/iceberg/blob/master/flink/v1.15/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkIcebergSink.java).
+The iceberg API also allows users to write generic `DataStream<T>` to iceberg table, more example could be found in this [unit test](https://github.com/apache/iceberg/blob/master/flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkIcebergSink.java).
### Overwrite data
-To overwrite the data in existing iceberg table dynamically, we could set the `overwrite` flag in FlinkSink builder.
+To overwrite the data in existing iceberg table dynamically, set the `overwrite` flag in FlinkSink builder.
Review Comment:
Yes, the sentence doesn't read very easily. I like your suggestion very much. Updated!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a diff in pull request #7070: Flink: Update the docs
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#discussion_r1133141168
##########
docs/flink-getting-started.md:
##########
@@ -344,11 +324,11 @@ CREATE TABLE `hive_catalog`.`default`.`sample` (
);
```
-Table create commands support the most commonly used [flink create clauses](https://ci.apache.org/projects/flink/flink-docs-release-1.11/dev/table/sql/create.html#create-table) now, including:
+Table create commands support the commonly used [Flink create clauses](https://nightlies.apache.org/flink/flink-docs-release-1.16/docs/dev/table/sql/create/) including:
Review Comment:
Good one, the url seems to be stable
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on a diff in pull request #7070: Flink: Update the docs
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#discussion_r1132940706
##########
docs/flink-getting-started.md:
##########
@@ -344,11 +324,11 @@ CREATE TABLE `hive_catalog`.`default`.`sample` (
);
```
-Table create commands support the most commonly used [flink create clauses](https://ci.apache.org/projects/flink/flink-docs-release-1.11/dev/table/sql/create.html#create-table) now, including:
+Table create commands support the commonly used [Flink create clauses](https://nightlies.apache.org/flink/flink-docs-release-1.16/docs/dev/table/sql/create/) including:
Review Comment:
what about using the doc link from master branch? then we don't need to worry about the Flink versions in the doc
https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/create/
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu merged pull request #7070: Flink: Update the docs
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu merged PR #7070:
URL: https://github.com/apache/iceberg/pull/7070
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on a diff in pull request #7070: Flink: Update the docs
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#discussion_r1132942620
##########
docs/flink-getting-started.md:
##########
@@ -687,11 +664,11 @@ FlinkSink.forRowData(input)
env.execute("Test Iceberg DataStream");
```
-The iceberg API also allows users to write generic `DataStream<T>` to iceberg table, more example could be found in this [unit test](https://github.com/apache/iceberg/blob/master/flink/v1.15/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkIcebergSink.java).
+The iceberg API also allows users to write generic `DataStream<T>` to iceberg table, more example could be found in this [unit test](https://github.com/apache/iceberg/blob/master/flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkIcebergSink.java).
### Overwrite data
-To overwrite the data in existing iceberg table dynamically, we could set the `overwrite` flag in FlinkSink builder.
+To overwrite the data in existing iceberg table dynamically, set the `overwrite` flag in FlinkSink builder.
Review Comment:
is this a complete sentence? seems missing noun :) Maybe reverse the order like you did in some other places. `Set the overwrite flag ... to overwrite data in the existing partitions.`?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on pull request #7070: Flink: Update the docs
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#issuecomment-1465127391
Any time @stevenzwu thanks for the prompt review!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on a diff in pull request #7070: Flink: Update the docs
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#discussion_r1132920464
##########
docs/flink-getting-started.md:
##########
@@ -46,156 +46,136 @@ Apache Iceberg supports both [Apache Flink](https://flink.apache.org/)'s DataStr
## Preparation when using Flink SQL Client
-To create iceberg table in flink, we recommend to use [Flink SQL Client](https://ci.apache.org/projects/flink/flink-docs-stable/dev/table/sqlClient.html) because it's easier for users to understand the concepts.
+To create Iceberg table in Flink, recommended is to use [Flink SQL Client](https://ci.apache.org/projects/flink/flink-docs-stable/dev/table/sqlClient.html) as it's easier for users to understand the concepts.
-Step.1 Downloading the flink 1.11.x binary package from the apache flink [download page](https://flink.apache.org/downloads.html). We now use scala 2.12 to archive the apache iceberg-flink-runtime jar, so it's recommended to use flink 1.11 bundled with scala 2.12.
+Download the Flink 1.16.1 binary package from the Apache Flink [download page](https://flink.apache.org/downloads.html). Iceberg uses Scala 2.12 when compiling the Apache `iceberg-flink-runtime` jar, so it's recommended to use Flink 1.16 bundled with Scala 2.12.
Review Comment:
let's remove the version from this sentence
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a diff in pull request #7070: Flink: Update the docs
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#discussion_r1133141036
##########
docs/flink-getting-started.md:
##########
@@ -46,156 +46,136 @@ Apache Iceberg supports both [Apache Flink](https://flink.apache.org/)'s DataStr
## Preparation when using Flink SQL Client
-To create iceberg table in flink, we recommend to use [Flink SQL Client](https://ci.apache.org/projects/flink/flink-docs-stable/dev/table/sqlClient.html) because it's easier for users to understand the concepts.
+To create Iceberg table in Flink, recommended is to use [Flink SQL Client](https://ci.apache.org/projects/flink/flink-docs-stable/dev/table/sqlClient.html) as it's easier for users to understand the concepts.
Review Comment:
Oops, refactored that one too many times, thanks!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on pull request #7070: Flink: Update the docs
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#issuecomment-1465059092
thanks @Fokko for updating the doc
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on a diff in pull request #7070: Flink: Update the docs
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#discussion_r1132919828
##########
docs/flink-getting-started.md:
##########
@@ -46,156 +46,136 @@ Apache Iceberg supports both [Apache Flink](https://flink.apache.org/)'s DataStr
## Preparation when using Flink SQL Client
-To create iceberg table in flink, we recommend to use [Flink SQL Client](https://ci.apache.org/projects/flink/flink-docs-stable/dev/table/sqlClient.html) because it's easier for users to understand the concepts.
+To create Iceberg table in Flink, recommended is to use [Flink SQL Client](https://ci.apache.org/projects/flink/flink-docs-stable/dev/table/sqlClient.html) as it's easier for users to understand the concepts.
Review Comment:
`recommended` -> `recommended way` or `it is recommended to`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on a diff in pull request #7070: Flink: Update the docs
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#discussion_r1133011026
##########
docs/flink-getting-started.md:
##########
@@ -747,8 +724,8 @@ DataStream<org.apache.avro.generic.GenericRecord> dataStream = ...;
Schema icebergSchema = table.schema();
-// if the Iceberg table schema contains time fields, we can't use
-// Avro schema converted from Iceberg schema via AvroSchemaUtil.
+// if the Iceberg table schema contains time fields, and can't use
+// the Avro schema converted from Iceberg schema via AvroSchemaUtil.
Review Comment:
nit: seems odd syntax. maybe `the Avro schema converted from Iceberg schema can't be used due to precision difference between how Iceberg schema (micro) and Flink AvroToRowDataConverters (milli) deal with time type`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on a diff in pull request #7070: Flink: Update the docs
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #7070:
URL: https://github.com/apache/iceberg/pull/7070#discussion_r1133011026
##########
docs/flink-getting-started.md:
##########
@@ -747,8 +724,8 @@ DataStream<org.apache.avro.generic.GenericRecord> dataStream = ...;
Schema icebergSchema = table.schema();
-// if the Iceberg table schema contains time fields, we can't use
-// Avro schema converted from Iceberg schema via AvroSchemaUtil.
+// if the Iceberg table schema contains time fields, and can't use
+// the Avro schema converted from Iceberg schema via AvroSchemaUtil.
Review Comment:
nit: seems odd syntax. maybe `the Avro schema converted from Iceberg schema can't be used due to precision difference between how Iceberg schema (micro) and Flink {@link AvroToRowDataConverters} (milli) deal with time type`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org