You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by GitBox <gi...@apache.org> on 2021/08/02 06:08:59 UTC
[GitHub] [zeppelin] zjffdu opened a new pull request #4195: save
zjffdu opened a new pull request #4195:
URL: https://github.com/apache/zeppelin/pull/4195
### What is this PR for?
A few sentences describing the overall goals of the pull request's commits.
First time? Check out the contributing guide - https://zeppelin.apache.org/contribution/contributions.html
### What type of PR is it?
[Bug Fix | Improvement | Feature | Documentation | Hot Fix | Refactoring]
### Todos
* [ ] - Task
### What is the Jira issue?
* Open an issue on Jira https://issues.apache.org/jira/browse/ZEPPELIN/
* Put link here, and add [ZEPPELIN-*Jira number*] in PR title, eg. [ZEPPELIN-533]
### How should this be tested?
* Strongly recommended: add automated unit tests for any new or changed behavior
* Outline any manual steps to test the PR here.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update?
* Is there breaking changes for older versions?
* Does this needs documentation?
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on a change in pull request #4195: [WIP][ZEPPELIN-5469]
Posted by GitBox <gi...@apache.org>.
zjffdu commented on a change in pull request #4195:
URL: https://github.com/apache/zeppelin/pull/4195#discussion_r683932041
##########
File path: flink/flink1.10-shims/src/main/java/org/apache/zeppelin/flink/Flink110Shims.java
##########
@@ -337,4 +344,27 @@ public Map extractTableConfigOptions() {
}
return fields;
}
+
+ public boolean isTimeIndicatorType(Object type) {
+ return FlinkTypeFactory.isTimeIndicatorType((TypeInformation<?>) type);
+ }
+
+ @Override
+ public Object lookupExecutor(ClassLoader classLoader, Object environmentSettings, Object sEnv) {
Review comment:
Yes, if you take a look at the pom.xml under `flink-shims` module, there's no flink dependency, because we can not specify flink dependency here, it leave its child module to decide which flink version to use.
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on pull request #4195: [ZEPPELIN-5469] Support Flink 1.14
Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4195:
URL: https://github.com/apache/zeppelin/pull/4195#issuecomment-934017379
Will merge if no more comment
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on pull request #4195: [ZEPPELIN-5469] Support Flink 1.14
Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4195:
URL: https://github.com/apache/zeppelin/pull/4195#issuecomment-933448231
> Maybe it is also time to remove old Flink versions. Based on https://flink.apache.org/downloads.html only 1.12.x, 1.13.x and 1.14.x are the stable versions.
That's right, this PR is for the next minor release of Zeppelin: 0.10.1, so I will keep compatibility, all versions of flink will be supported. I plan to remove the old version support in the next major release: 0.11.0. I will do it in another ticket.
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] cuspymd commented on a change in pull request #4195: [WIP][ZEPPELIN-5469]
Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4195:
URL: https://github.com/apache/zeppelin/pull/4195#discussion_r680805316
##########
File path: flink/flink1.10-shims/src/main/java/org/apache/zeppelin/flink/Flink110Shims.java
##########
@@ -337,4 +344,27 @@ public Map extractTableConfigOptions() {
}
return fields;
}
+
+ public boolean isTimeIndicatorType(Object type) {
+ return FlinkTypeFactory.isTimeIndicatorType((TypeInformation<?>) type);
+ }
+
+ @Override
+ public Object lookupExecutor(ClassLoader classLoader, Object environmentSettings, Object sEnv) {
Review comment:
Every implementation of `isTimeIndicatorType()`, `lookupExecutor()` seems be the same. Is it better move it to `FlinkShims` class?
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] jongyoul closed pull request #4195: [ZEPPELIN-5469] Support Flink 1.14
Posted by GitBox <gi...@apache.org>.
jongyoul closed pull request #4195:
URL: https://github.com/apache/zeppelin/pull/4195
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] cuspymd commented on a change in pull request #4195: [WIP][ZEPPELIN-5469]
Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4195:
URL: https://github.com/apache/zeppelin/pull/4195#discussion_r683931010
##########
File path: flink/flink1.10-shims/src/main/java/org/apache/zeppelin/flink/Flink110Shims.java
##########
@@ -337,4 +344,27 @@ public Map extractTableConfigOptions() {
}
return fields;
}
+
+ public boolean isTimeIndicatorType(Object type) {
+ return FlinkTypeFactory.isTimeIndicatorType((TypeInformation<?>) type);
+ }
+
+ @Override
+ public Object lookupExecutor(ClassLoader classLoader, Object environmentSettings, Object sEnv) {
Review comment:
As bellow, the class `Flink110Shims` inherits the class `FlinkShims`.
Do you mean the method `Flink110Shims::lookupExecutor()` can't be moved to `FlinkShims::lookupExecutor()` due to a dependency issue?
```
public class Flink110Shims extends FlinkShims
```
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] cuspymd commented on a change in pull request #4195: [WIP][ZEPPELIN-5469]
Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4195:
URL: https://github.com/apache/zeppelin/pull/4195#discussion_r684088748
##########
File path: flink/flink1.10-shims/src/main/java/org/apache/zeppelin/flink/Flink110Shims.java
##########
@@ -337,4 +344,27 @@ public Map extractTableConfigOptions() {
}
return fields;
}
+
+ public boolean isTimeIndicatorType(Object type) {
+ return FlinkTypeFactory.isTimeIndicatorType((TypeInformation<?>) type);
+ }
+
+ @Override
+ public Object lookupExecutor(ClassLoader classLoader, Object environmentSettings, Object sEnv) {
Review comment:
Thanks for the detailed explanation.
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on a change in pull request #4195: [WIP][ZEPPELIN-5469]
Posted by GitBox <gi...@apache.org>.
zjffdu commented on a change in pull request #4195:
URL: https://github.com/apache/zeppelin/pull/4195#discussion_r680811652
##########
File path: flink/flink1.10-shims/src/main/java/org/apache/zeppelin/flink/Flink110Shims.java
##########
@@ -337,4 +344,27 @@ public Map extractTableConfigOptions() {
}
return fields;
}
+
+ public boolean isTimeIndicatorType(Object type) {
+ return FlinkTypeFactory.isTimeIndicatorType((TypeInformation<?>) type);
+ }
+
+ @Override
+ public Object lookupExecutor(ClassLoader classLoader, Object environmentSettings, Object sEnv) {
Review comment:
flink-shims don't have flink dependencies, so can not use these flink classes. Only sub-modules has flink dependency. This do cause code duplication, but I don't have better way to avoid that.
--
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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on pull request #4195: [ZEPPELIN-5469] Support Flink 1.14
Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4195:
URL: https://github.com/apache/zeppelin/pull/4195#issuecomment-930856150
CI is passed, ready for 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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on pull request #4195: [ZEPPELIN-5469] Support Flink 1.14
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4195:
URL: https://github.com/apache/zeppelin/pull/4195#issuecomment-933388910
Maybe it is also time to remove old Flink versions.
Based on https://flink.apache.org/downloads.html only 1.12.x, 1.13.x and 1.14.x are the stable 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: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org