You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/09/14 17:10:02 UTC
[GitHub] [iceberg] Flyangz opened a new pull request #3116: upgrade to flink 1.13
Flyangz opened a new pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116
This job is completed on the basis of #2629 , so thanks for the contribution of @zhangjun0x01 .
Below are some explanations:
1. The options are not checked in `FlinkCatalogFactory.createCatalog()`, this is to be compatible with the old implementation. And this way we can use customized options as before without pre-settings.
2. The implementation is not compatible with Flink 1.12 because CatalogFactory interface does not extend Factory interface in 1.12.
@openinx @kbendick
--
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] openinx commented on pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-927246114
Okay, so does any other problems for guaranteeing compatibility for both flink 1.12 & flink 1.13 in your own context ? I think we will need to make the whole unit tests work for both flink 1.12 & flink 1.13 , the work should be similar to this PR: https://github.com/apache/iceberg/commit/111fe810c81eb2fea7d849ecc21daedd1c429355. I think it's good to make it into a separate PR !
--
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] Flyangz commented on a change in pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
Flyangz commented on a change in pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#discussion_r715991358
##########
File path: flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactory.java
##########
@@ -101,11 +108,28 @@ static CatalogLoader createCatalogLoader(String name, Map<String, String> proper
}
}
+ @Override
+ public String factoryIdentifier() {
+ return IDENTIFIER;
+ }
+
+ @Override
+ public Set<ConfigOption<?>> requiredOptions() {
+ return Sets.newHashSet();
+ }
+
+ @Override
+ public Set<ConfigOption<?>> optionalOptions() {
+ final Set<ConfigOption<?>> options = Sets.newHashSet();
+ options.add(FactoryUtil.PROPERTY_VERSION);
+ return options;
+ }
+
@Override
public Map<String, String> requiredContext() {
Review comment:
As [`Context`](https://github.com/apache/flink/blob/5f007ff6c8224c6e5a14de2c79296eb85a22fe1f/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java#L66) class is missing in 1.12, I think we can only implement these deprecated methods.
--
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] wg1026688210 commented on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
wg1026688210 commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-919723916
we do not have the plan of improving flink to 1.13 recently, is there a way to support both 1.12 and 1.13 versions.
--
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] kbendick commented on a change in pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#discussion_r711340777
##########
File path: versions.props
##########
@@ -1,7 +1,7 @@
org.slf4j:* = 1.7.25
org.apache.avro:avro = 1.10.1
org.apache.calcite:* = 1.10.0
Review comment:
Do we know if our calcite version needs to be upgraded?
--
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] openinx commented on pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-924712667
@kbendick @Flyangz According to the last discussion from [mail list ](https://lists.apache.org/x/thread.html/r2b93b991efb7a584438ffda8d29c1dc6de27ec8fc3293636aeb1b092@%3Cdev.iceberg.apache.org%3E), I think we'd better to keep the same strategy as the apache spark plan to do, users from apache iceberg community also proposed to support both flink 1.12 & flink 1.13. Since the difference between flink 1.12 & 1.13 is not so large, I think it's possible to support both of them in the single flink module ( I mean we don't need to separate them into two different modules).
--
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] kbendick commented on a change in pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#discussion_r711330213
##########
File path: flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactory.java
##########
@@ -101,11 +108,28 @@ static CatalogLoader createCatalogLoader(String name, Map<String, String> proper
}
}
+ @Override
+ public String factoryIdentifier() {
+ return IDENTIFIER;
+ }
+
+ @Override
+ public Set<ConfigOption<?>> requiredOptions() {
+ return Sets.newHashSet();
+ }
+
+ @Override
+ public Set<ConfigOption<?>> optionalOptions() {
+ final Set<ConfigOption<?>> options = Sets.newHashSet();
+ options.add(FactoryUtil.PROPERTY_VERSION);
+ return options;
+ }
+
@Override
public Map<String, String> requiredContext() {
Map<String, String> context = Maps.newHashMap();
- context.put(CatalogDescriptorValidator.CATALOG_TYPE, "iceberg");
- context.put(CatalogDescriptorValidator.CATALOG_PROPERTY_VERSION, "1");
+ context.put(TYPE, "iceberg");
+ context.put(PROPERTY_VERSION, "1");
Review comment:
I don't think that this config value should be required anymore (as people should be allowed to create V2 tables).
--
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] kbendick commented on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-922070927
> This committed code implements both `TableFactory` and `Factory`. [Actually, Flink 1.13 will continue to use `TableFactory` API](https://github.com/apache/flink/blob/5f007ff6c8224c6e5a14de2c79296eb85a22fe1f/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/FactoryUtil.java#L240). So only implementing `TableFactory` is the method with the least changes and the lowest risk to upgrade to 1.13. Which way do you prefer? Implementing both or just `TableFactory`?
Oh that's rather unfortunate. I had hoped that Flink 1.13 would use the new interface, if present, while allowing for the old interface to also be implemented for backwards compatibility.
Perhaps somebody more involved with the Flink Community than myself can comment as to whether or not there might be a flag added or that could be added to tell Flink to prefer the new interface first (when present).
I would suggest implementing both interfaces for now (and also organizing the interface functions so that they're organized together based on which interface), so as to keep support for Flink 1.12.
While this won't give us the benefit of the new API, we can upgrade to Flink 1.13.2 and then consider the importance of that later on. I would still implement the new API functions though (since it's already been done), and then we can track that in a follow up issue. We might indeed decide dropping support for 1.12 is worth it, but we can't make that decision on our own.
--
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] Flyangz commented on a change in pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
Flyangz commented on a change in pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#discussion_r715989997
##########
File path: api/src/test/java/org/apache/iceberg/AssertHelpers.java
##########
@@ -155,4 +155,18 @@ public static void assertEmptyAvroField(GenericRecord record, String field) {
AvroRuntimeException.class,
() -> record.get(field));
}
+
+ /**
+ * Same as {@link AssertHelpers#assertThrowsCause}, but this method compares root cause.
+ */
+ public static void assertThrowsRootCause(String message,
+ Class<? extends Exception> expected,
+ String containedInMessage,
+ Runnable runnable) {
Review comment:
fixed 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: 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] Flyangz commented on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
Flyangz commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-921912251
This committed code implements both `TableFactory` and `Factory`. [Actually, Flink 1.13 would continue to use `TableFactory` API](https://github.com/apache/flink/blob/5f007ff6c8224c6e5a14de2c79296eb85a22fe1f/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/FactoryUtil.java#L240). So only implementing `TableFactory` is the method with the least changes and the lowest risk to upgrade to 1.13. Which way do you prefer? Implementing both or just `TableFactory`?
--
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] openinx commented on a change in pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#discussion_r713702839
##########
File path: flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactory.java
##########
@@ -101,11 +108,28 @@ static CatalogLoader createCatalogLoader(String name, Map<String, String> proper
}
}
+ @Override
+ public String factoryIdentifier() {
+ return IDENTIFIER;
+ }
+
+ @Override
+ public Set<ConfigOption<?>> requiredOptions() {
+ return Sets.newHashSet();
+ }
+
+ @Override
+ public Set<ConfigOption<?>> optionalOptions() {
+ final Set<ConfigOption<?>> options = Sets.newHashSet();
+ options.add(FactoryUtil.PROPERTY_VERSION);
+ return options;
+ }
+
@Override
public Map<String, String> requiredContext() {
Map<String, String> context = Maps.newHashMap();
- context.put(CatalogDescriptorValidator.CATALOG_TYPE, "iceberg");
- context.put(CatalogDescriptorValidator.CATALOG_PROPERTY_VERSION, "1");
+ context.put(TYPE, "iceberg");
+ context.put(PROPERTY_VERSION, "1");
Review comment:
@kbendick this `property-version` is indicating the flink connector's properties version, not the apache iceberg table format, which is used for guarantee the properties compatibility.
--
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] kbendick edited a comment on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
kbendick edited a comment on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-919423650
> 1. The options are not checked in `FlinkCatalogFactory.createCatalog()`, this is to be compatible with the old implementation. And this way we can use customized options as before without pre-settings.
+1 to this. We have a number of customized options for things like AWS s3 / IAM etc that go on catalogs and we need to be able to support them.
> 2. The implementation is not compatible with Flink 1.12 because CatalogFactory interface does not extend Factory interface in 1.12.
Does this mean we're dropping support entirely for Flink 1.12 in the next release (and effectively in the master branch)? This seems potentially premature. [I thought that the Flink 1.13 interface added `Factory`, but kept the old interface `TableFactory` as well that was extended in 1.12 for backwards compatibility.](https://github.com/apache/flink/blob/f74db4b6cce9a28f484dc9d5e6470a50b15c121a/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java#L40)
Is it possible we can do the same (continue to implement both interfaces or have two separate classes) to keep support for Flink 1.12 @Flyangz?
--
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] Flyangz commented on pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
Flyangz commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-927009732
In order to be compatible with Flink 1.12 and 1.13, this commit only implements `TableFactory` interface. By 'compatible', I mean the code can be compiled with both 1.12 and 1.13 and pass all unit tests in iceberg-flink module.
I have tried to make iceberg to use different META-INF file based on Flink version, but this still does not work cause the [`createCatalog(Context context)` method in `Factory`](https://github.com/apache/flink/blob/5f007ff6c8224c6e5a14de2c79296eb85a22fe1f/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java#L61). The `Context` object is new in 1.13. Are there any feasible way that we can compile iceberg depended on 1.12 without this class?
@kbendick @openinx
--
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] Flyangz commented on pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
Flyangz commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-927242283
> > In order to be compatible with Flink 1.12 and 1.13, this commit only implements `TableFactory` interface. By 'compatible', I mean the code can be compiled with both 1.12 and 1.13 and pass all unit tests in iceberg-flink module.
> > I have tried to make iceberg to use different META-INF file based on Flink version, but this still does not work cause the [`createCatalog(Context context)` method in `Factory`](https://github.com/apache/flink/blob/5f007ff6c8224c6e5a14de2c79296eb85a22fe1f/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java#L61). The `Context` object is new in 1.13. Are there any feasible way that we can compile iceberg depended on 1.12 without this class?
> > @kbendick @openinx
>
> Is the deprecated [createCatalog](https://github.com/apache/flink/blob/5f007ff6c8224c6e5a14de2c79296eb85a22fe1f/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java#L51) work for both flink 1.12 & flink 1.13 ?
I think so, at least it can pass all the unit tests in the iceberg-flink module compiled with 1.12 or 1.13.
In 1.13, as I mentioned above, [Flink will use deprecated createCatalog first](https://github.com/apache/flink/blob/5f007ff6c8224c6e5a14de2c79296eb85a22fe1f/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/FactoryUtil.java#L240) for compatibility. In 1.12, this method is not deprecated and has already been used in current iceberg master code.
--
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] Flyangz commented on pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
Flyangz commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-927250618
> Okay, so does any other problems for guaranteeing compatibility for both flink 1.12 & flink 1.13 in your own context ? I think we will need to make the whole unit tests work for both flink 1.12 & flink 1.13 , the work should be similar to this PR: [111fe81](https://github.com/apache/iceberg/commit/111fe810c81eb2fea7d849ecc21daedd1c429355). I think it's good to make it into a separate PR !
1. No.
2. Yes, integration testing is a more reliable way to check compatibility.
--
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] openinx commented on pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-928653449
@Flyangz Yes, this's the correct issue. You can just assign it to yourself.
--
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] Flyangz commented on pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
Flyangz commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-927654206
> @Flyangz Please file a new issue to address the unit tests things, and attach the issue to [[Priority 1] Flink: Upgrade to 1.13.2](https://github.com/apache/iceberg/projects/12#card-69156027) project.
Sorry for the late reply. Do you mean #3183 issue that you have already created ?
--
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] openinx commented on a change in pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#discussion_r713565775
##########
File path: api/src/test/java/org/apache/iceberg/AssertHelpers.java
##########
@@ -155,4 +155,18 @@ public static void assertEmptyAvroField(GenericRecord record, String field) {
AvroRuntimeException.class,
() -> record.get(field));
}
+
+ /**
+ * Same as {@link AssertHelpers#assertThrowsCause}, but this method compares root cause.
+ */
+ public static void assertThrowsRootCause(String message,
+ Class<? extends Exception> expected,
+ String containedInMessage,
+ Runnable runnable) {
Review comment:
Nit: It's not the correct iceberg indent, I think you will need to configure your IDE by following this: https://iceberg.apache.org/community/#setting-up-ide-and-code-style
--
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] Flyangz edited a comment on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
Flyangz edited a comment on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-921912251
This committed code implements both `TableFactory` and `Factory`. [Actually, Flink 1.13 will continue to use `TableFactory` API](https://github.com/apache/flink/blob/5f007ff6c8224c6e5a14de2c79296eb85a22fe1f/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/FactoryUtil.java#L240). So only implementing `TableFactory` is the method with the least changes and the lowest risk to upgrade to 1.13. Which way do you prefer? Implementing both or just `TableFactory`?
--
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] openinx commented on pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-927255419
@Flyangz Please file a new issue to address the unit tests things, and attach the issue to [[Priority 1] Flink: Upgrade to 1.13.2](https://github.com/apache/iceberg/projects/12#card-69156027) project.
--
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] kbendick commented on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-919423650
> 1. The options are not checked in `FlinkCatalogFactory.createCatalog()`, this is to be compatible with the old implementation. And this way we can use customized options as before without pre-settings.
+1 to this. We have a number of customized options for things like AWS s3 / IAM etc that go on catalogs and we need to be able to support them.
> 2. The implementation is not compatible with Flink 1.12 because CatalogFactory interface does not extend Factory interface in 1.12.
Does this mean we're dropping support entirely for Flink 1.12 in the next release (and effectively in the master branch)? This seems potentially premature. [I thought that the Flink 1.13 interface added `Factory`, but kept the old interface `CatalogFactory` as well that was extended in 1.12 for backwards compatibility.](https://github.com/apache/flink/blob/f74db4b6cce9a28f484dc9d5e6470a50b15c121a/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java#L40)
Is it possible we can do the same (continue to implement both interfaces or have two separate classes) to keep support for Flink 1.12 @Flyangz?
--
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] openinx merged pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx merged pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116
--
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] Flyangz commented on a change in pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
Flyangz commented on a change in pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#discussion_r715990954
##########
File path: flink/src/main/resources/META-INF/services/org.apache.flink.table.factories.Factory
##########
@@ -13,4 +13,5 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-org.apache.iceberg.flink.FlinkDynamicTableFactory
\ No newline at end of file
+org.apache.iceberg.flink.FlinkDynamicTableFactory
+org.apache.iceberg.flink.FlinkCatalogFactory
Review comment:
If we only support 1.13, then is ok to remove it. But if we want to be compatible with 1.12, `FlinkCatalogFactory` need to be treated as `TableFactory` because `Context` class is missing in 1.12 and without implementing `createCatalog(Context context)` will throw exception when we place `FlinkCatalogFactory` in [ org.apache.flink.table.factories.Factory](https://github.com/apache/iceberg/blob/master/flink/src/main/resources/META-INF/services/org.apache.flink.table.factories.Factory)
--
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] Flyangz commented on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
Flyangz commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-921782417
How about this time? @kbendick It can support both 1.12 and 1.13, though it implements deprecated methods([`requiredContext`, `supportedProperties` and `supportedProperties` marked deprecated at 1.13](https://github.com/apache/flink/blob/release-1.13.2/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java)).
@zhangjun0x01's original PR does not support 1.12, because that PR implements `Factory` interface API, like `factoryIdentifier` and `requiredOptions`. But [1.12's CatalogFactory does not extend `Factory`](https://github.com/apache/flink/blob/release-1.12.1/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java).
--
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] Flyangz edited a comment on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
Flyangz edited a comment on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-921782417
How about this time? @kbendick It can support both 1.12 and 1.13, though it implements deprecated methods([`requiredContext`, `supportedProperties` and `supportedProperties` marked deprecated at 1.13](https://github.com/apache/flink/blob/release-1.13.2/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java)).
@zhangjun0x01's original PR does not support 1.12, because that PR implements `Factory` interface API, like `factoryIdentifier`, and [1.12's CatalogFactory does not extend `Factory`](https://github.com/apache/flink/blob/release-1.12.1/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java).
--
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] kbendick edited a comment on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
kbendick edited a comment on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-922070927
> This committed code implements both `TableFactory` and `Factory`. [Actually, Flink 1.13 will continue to use `TableFactory` API](https://github.com/apache/flink/blob/5f007ff6c8224c6e5a14de2c79296eb85a22fe1f/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/FactoryUtil.java#L240). So only implementing `TableFactory` is the method with the least changes and the lowest risk to upgrade to 1.13. Which way do you prefer? Implementing both or just `TableFactory`?
Oh that's rather unfortunate. I had hoped that Flink 1.13 would use the new interface, if present, while allowing for the old interface to also be implemented for backwards compatibility.
Perhaps somebody more involved with the Flink Community than myself can comment as to whether or not there might be a flag added or that could be added to tell Flink to prefer the new interface first (when present).
I would suggest implementing both interfaces for now (and also organizing the interface functions so that they're organized together based on which interface), so as to keep support for Flink 1.12.
While this won't give us the benefit of the new API, we can upgrade to Flink 1.13.2 and then consider the importance of that later on. I would still implement the new API functions though (since it's already been done), and then we can track that in a follow up issue. We might indeed decide dropping support for 1.12 is worth it, but we can't make that decision on our own.
EDIT: Perhaps it can be broken up into two different classes, and then when we build the jar only one META-INF file is placed on the class path depending on the Flink version. Seems really messy, but possibly we can consider this down the road.
Any thoughts on this @stevenzwu @openinx?
--
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] Flyangz removed a comment on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
Flyangz removed a comment on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-921782417
How about this time? @kbendick It can support both 1.12 and 1.13, though it implements deprecated methods([`requiredContext`, `supportedProperties` and `supportedProperties` marked deprecated at 1.13](https://github.com/apache/flink/blob/release-1.13.2/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java)).
@zhangjun0x01's original PR does not support 1.12, because that PR implements `Factory` interface API, like `factoryIdentifier`, and [1.12's CatalogFactory does not extend `Factory`](https://github.com/apache/flink/blob/release-1.12.1/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java).
--
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] openinx commented on a change in pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#discussion_r713713376
##########
File path: flink/src/main/resources/META-INF/services/org.apache.flink.table.factories.Factory
##########
@@ -13,4 +13,5 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-org.apache.iceberg.flink.FlinkDynamicTableFactory
\ No newline at end of file
+org.apache.iceberg.flink.FlinkDynamicTableFactory
+org.apache.iceberg.flink.FlinkCatalogFactory
Review comment:
Should we remove this file [org.apache.flink.table.factories.TableFactory)](https://github.com/apache/iceberg/blob/master/flink/src/main/resources/META-INF/services/org.apache.flink.table.factories.TableFactory) if we plan to upgrade the flink version to 1.13.2 directly ?
--
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] kbendick edited a comment on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
kbendick edited a comment on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-919725987
> we do not have the plan of improving flink to 1.13 recently, is there a way to support both 1.12 and 1.13 versions.
I believe that @zhangjun0x01's original PR supported both flink 1.12 and flink 1.13. Like I mentioned, if the `FlinkCatalogFactory` implements both APIs, will that not suffice to support both 1.12 and 1.13?
Dropping support for Flink 1.12 is a big decision in my opinion, and should be brought up on the mailing list or at the community sync-up.
Does my idea of continuing to implement `TableFactory`, in addition to adding the newer `Factory` interface functions, just like [Flink's `CatalogFactory` does itself ](https://github.com/apache/flink/blob/f74db4b6cce9a28f484dc9d5e6470a50b15c121a/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java#L40) not work? I thought that was why Flink's `CatalogFactory` implemented both the old interface and the new interface, to mark the old interface as deprecated but still support both versions.
--
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] openinx commented on pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-928653449
@Flyangz Yes, this's the correct issue. You can just assign it to yourself.
--
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] kbendick removed a comment on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
kbendick removed a comment on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-919421113
> 1. The options are not checked in `FlinkCatalogFactory.createCatalog()`, this is to be compatible with the old implementation. And this way we can use customized options as before without pre-settings.
+1 to this. We have a number of customized options for things like AWS s3 / IAM etc that go on catalogs and we need to be able to support them.
> 2. The implementation is not compatible with Flink 1.12 because CatalogFactory interface does not extend Factory interface in 1.12.
Does this mean we're dropping support entirely for Flink 1.12 in the next release (and effectively in the master branch)? This seems potentially premature. I thought that the Flink 1.13 interface added `Factory`, but kept the old interface that was extended in 1.12 for backwards compatibility.
Is it possible we can do the same @Flyangz?
--
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] kbendick commented on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-919725987
> we do not have the plan of improving flink to 1.13 recently, is there a way to support both 1.12 and 1.13 versions.
I believe that @zhangjun0x01's original PR supported both flink 1.12 and flink 1.13. Like I mentioned, if the `FlinkCatalogFactory` implements both APIs, will that not suffice to support both 1.12 and 1.13?
Dropping support for Flink 1.12 is a big decision in my opinion, and should be brought up on the mailing list or at the community sync-up.
Does my idea of continuing to implement both `Factory` and `TableFactory`, just like [Flink's `CatalogFactory` does itself ](https://github.com/apache/flink/blob/f74db4b6cce9a28f484dc9d5e6470a50b15c121a/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java#L40) not work? I thought that was why Flink's `CatalogFactory` implemented both the old interface and the new interface, to mark the old interface as deprecated but still support both versions.
--
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] openinx commented on a change in pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#discussion_r713707793
##########
File path: flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactory.java
##########
@@ -101,11 +108,28 @@ static CatalogLoader createCatalogLoader(String name, Map<String, String> proper
}
}
+ @Override
+ public String factoryIdentifier() {
+ return IDENTIFIER;
+ }
+
+ @Override
+ public Set<ConfigOption<?>> requiredOptions() {
+ return Sets.newHashSet();
+ }
+
+ @Override
+ public Set<ConfigOption<?>> optionalOptions() {
+ final Set<ConfigOption<?>> options = Sets.newHashSet();
+ options.add(FactoryUtil.PROPERTY_VERSION);
+ return options;
+ }
+
@Override
public Map<String, String> requiredContext() {
Review comment:
As the `requiredContext` and `supportedProperties` have been marked as `deprecated`, I will suggest to implement those in `requiredOptions` & `optionalOptions`.
--
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] Flyangz closed pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
Flyangz closed pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116
--
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] kbendick commented on pull request #3116: upgrade to flink 1.13
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-919421113
> 1. The options are not checked in `FlinkCatalogFactory.createCatalog()`, this is to be compatible with the old implementation. And this way we can use customized options as before without pre-settings.
+1 to this. We have a number of customized options for things like AWS s3 / IAM etc that go on catalogs and we need to be able to support them.
> 2. The implementation is not compatible with Flink 1.12 because CatalogFactory interface does not extend Factory interface in 1.12.
Does this mean we're dropping support entirely for Flink 1.12 in the next release (and effectively in the master branch)? This seems potentially premature. I thought that the Flink 1.13 interface added `Factory`, but kept the old interface that was extended in 1.12 for backwards compatibility.
Is it possible we can do the same @Flyangz?
--
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] openinx commented on a change in pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#discussion_r713715548
##########
File path: versions.props
##########
@@ -1,7 +1,7 @@
org.slf4j:* = 1.7.25
org.apache.avro:avro = 1.10.1
org.apache.calcite:* = 1.10.0
Review comment:
The apache flink runtime won't depend on this calcite jar , this calcite was introduced for [hive](https://github.com/apache/iceberg/pull/2110/files) before. So I think we don't need to change this calcite version for flink upgrading.
--
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] openinx commented on pull request #3116: Flink: Upgrade to flink 1.13.2
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3116:
URL: https://github.com/apache/iceberg/pull/3116#issuecomment-927232962
> In order to be compatible with Flink 1.12 and 1.13, this commit only implements `TableFactory` interface. By 'compatible', I mean the code can be compiled with both 1.12 and 1.13 and pass all unit tests in iceberg-flink module.
> I have tried to make iceberg to use different META-INF file based on Flink version, but this still does not work cause the [`createCatalog(Context context)` method in `Factory`](https://github.com/apache/flink/blob/5f007ff6c8224c6e5a14de2c79296eb85a22fe1f/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java#L61). The `Context` object is new in 1.13. Are there any feasible way that we can compile iceberg depended on 1.12 without this class?
> @kbendick @openinx
Is the deprecated [createCatalog](https://github.com/apache/flink/blob/5f007ff6c8224c6e5a14de2c79296eb85a22fe1f/flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/CatalogFactory.java#L51) work for both flink 1.12 & flink 1.13 ?
--
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