ahmedabu98 commented on code in PR #37539:
URL: https://github.com/apache/beam/pull/37539#discussion_r3190935972
##########
sdks/java/extensions/sql/iceberg/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/iceberg/IcebergTable.java:
##########
@@ -71,9 +74,21 @@ class IcebergTable extends SchemaBaseBeamTable {
this.tableIdentifier = tableIdentifier;
this.catalogConfig = catalogConfig;
ObjectNode properties = table.getProperties();
- if (properties.has(TRIGGERING_FREQUENCY_FIELD)) {
- this.triggeringFrequency =
properties.get(TRIGGERING_FREQUENCY_FIELD).asInt();
+ for (Map.Entry<String, JsonNode> property : properties.properties()) {
+ String name = property.getKey();
+ if (name.startsWith(BEAM_WRITE_PROPERTY)) {
+ String prop = name.substring(BEAM_WRITE_PROPERTY.length());
+ if (prop.equals(TRIGGERING_FREQUENCY_FIELD)) {
+ this.triggeringFrequency = property.getValue().asInt();
+ } else {
+ throw new IllegalArgumentException("Unknown Beam write property: " +
name);
Review Comment:
I'd rather fail and avoid a situation where the transform is behaving
unexpectedly.
I remember running into user issues with some IOs for the same reason, where
incorrect configurations were logged but the pipeline kept running.
##########
sdks/java/extensions/sql/iceberg/src/main/java/org/apache/beam/sdk/extensions/sql/meta/provider/iceberg/IcebergTable.java:
##########
@@ -71,9 +74,21 @@ class IcebergTable extends SchemaBaseBeamTable {
this.tableIdentifier = tableIdentifier;
this.catalogConfig = catalogConfig;
ObjectNode properties = table.getProperties();
- if (properties.has(TRIGGERING_FREQUENCY_FIELD)) {
- this.triggeringFrequency =
properties.get(TRIGGERING_FREQUENCY_FIELD).asInt();
+ for (Map.Entry<String, JsonNode> property : properties.properties()) {
+ String name = property.getKey();
+ if (name.startsWith(BEAM_WRITE_PROPERTY)) {
Review Comment:
Good point. Made it insensitive
##########
sdks/java/io/iceberg/src/main/java/org/apache/beam/sdk/io/iceberg/IcebergCatalogConfig.java:
##########
@@ -162,7 +162,11 @@ public void createTable(
icebergIdentifier,
icebergSchema,
icebergSpec);
- catalog().createTable(icebergIdentifier, icebergSchema, icebergSpec,
properties);
+ if (properties != null) {
Review Comment:
Beam was complaining about passing a null value to the method signature,
which takes non-null values.
I switched to use the table builder format instead (was recently made aware
of it).
--
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]
