You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/05/09 20:21:45 UTC

[GitHub] [flink-ml] dotbg opened a new pull request, #95: Migrated to flink 1.15.0.

dotbg opened a new pull request, #95:
URL: https://github.com/apache/flink-ml/pull/95

   * Flink 1.15.0
   * Scala binary version used in test artifacts only.  Hence artifact names were changed


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] lindong28 closed pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
lindong28 closed pull request #95: Migrated to flink 1.15.0.
URL: https://github.com/apache/flink-ml/pull/95


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] yunfengzhou-hub commented on pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
yunfengzhou-hub commented on PR #95:
URL: https://github.com/apache/flink-ml/pull/95#issuecomment-1124460428

   I tried running the command above locally and there are still errors. Could you please make a double check on the correctness of the changes? 
   
   You can configure to run the Github Actions on your forked repo. For example, Here are Actions executed on my PRs. 
   https://github.com/yunfengzhou-hub/flink-ml/actions


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] dotbg commented on pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
dotbg commented on PR #95:
URL: https://github.com/apache/flink-ml/pull/95#issuecomment-1122249547

   @yunfengzhou-hub sure. 


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] dotbg commented on pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
dotbg commented on PR #95:
URL: https://github.com/apache/flink-ml/pull/95#issuecomment-1131286644

   @yunfengzhou-hub thanks a lot for sharing. I've got it green https://github.com/dotbg/flink-ml/actions/runs/2348656481


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] lindong28 commented on pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
lindong28 commented on PR #95:
URL: https://github.com/apache/flink-ml/pull/95#issuecomment-1138548254

   Thanks @yunfengzhou-hub for reviewing the PR and for fixing the python test failures in https://github.com/apache/flink-ml/pull/104. LGTM.


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] dotbg commented on a diff in pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
dotbg commented on code in PR #95:
URL: https://github.com/apache/flink-ml/pull/95#discussion_r877976862


##########
flink-ml-lib/src/test/java/org/apache/flink/ml/clustering/OnlineKMeansTest.java:
##########
@@ -407,13 +415,15 @@ public void testSaveAndReload() throws Exception {
 
         transformAndOutputData(loadedOnlineModel);
 
-        miniCluster.submitJob(env.getStreamGraph().getJobGraph());
-        waitInitModelDataSetup();
+        final JobSubmissionResult jobSubmissionResult =
+                miniCluster.submitJob(env.getStreamGraph().getJobGraph()).get();
+        final JobID jobID = jobSubmissionResult.getJobID();

Review Comment:
   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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] yunfengzhou-hub commented on pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
yunfengzhou-hub commented on PR #95:
URL: https://github.com/apache/flink-ml/pull/95#issuecomment-1121813951

   Hi @dotbg , thanks for contributing to Flink ML.
   
   I tried running `mvn clean install` on this PR, and it seems that there are still errors during compilation. Could you please fix them and push to this PR again?


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] dotbg commented on a diff in pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
dotbg commented on code in PR #95:
URL: https://github.com/apache/flink-ml/pull/95#discussion_r877947355


##########
flink-ml-dist/pom.xml:
##########
@@ -19,14 +19,17 @@ under the License.
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
+    <properties>
+        <statefun-flink-core.version>3.2.0</statefun-flink-core.version>
+    </properties>

Review Comment:
   sure. Will do shortly



-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] yunfengzhou-hub commented on pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
yunfengzhou-hub commented on PR #95:
URL: https://github.com/apache/flink-ml/pull/95#issuecomment-1138262348

   Hi @dotbg , I have fixed the test failures of this PR, created #104 , and listed you as co-author.


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] yunfengzhou-hub commented on pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
yunfengzhou-hub commented on PR #95:
URL: https://github.com/apache/flink-ml/pull/95#issuecomment-1132696084

   Hi @dotbg , Could you please check the failures in the Python checks? So far as I can see they should just be caused by removing scala versions in package names.


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] dotbg commented on a diff in pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
dotbg commented on code in PR #95:
URL: https://github.com/apache/flink-ml/pull/95#discussion_r877950128


##########
flink-ml-lib/src/test/java/org/apache/flink/ml/clustering/OnlineKMeansTest.java:
##########
@@ -407,13 +415,15 @@ public void testSaveAndReload() throws Exception {
 
         transformAndOutputData(loadedOnlineModel);
 
-        miniCluster.submitJob(env.getStreamGraph().getJobGraph());
-        waitInitModelDataSetup();
+        final JobSubmissionResult jobSubmissionResult =
+                miniCluster.submitJob(env.getStreamGraph().getJobGraph()).get();
+        final JobID jobID = jobSubmissionResult.getJobID();

Review Comment:
   Generally, I'd like to avoid using `Future.get()` without a timeout. Is it ok to assume 1 second? 
   I could chain the test as well, but this may make the code way harder to follow. 



-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] dotbg commented on pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
dotbg commented on PR #95:
URL: https://github.com/apache/flink-ml/pull/95#issuecomment-1123587883

   @yunfengzhou-hub please, trigger the pipeline


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] dotbg commented on pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
dotbg commented on PR #95:
URL: https://github.com/apache/flink-ml/pull/95#issuecomment-1139500172

   thanks a lot @yunfengzhou-hub, @lindong28 . Sorry did not have time recently. 


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] lindong28 commented on pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
lindong28 commented on PR #95:
URL: https://github.com/apache/flink-ml/pull/95#issuecomment-1134036824

   Hey @dotbg, thanks for the PR! The PR looks good overall. Could you help fix the test failure?


-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink-ml] yunfengzhou-hub commented on a diff in pull request #95: Migrated to flink 1.15.0.

Posted by GitBox <gi...@apache.org>.
yunfengzhou-hub commented on code in PR #95:
URL: https://github.com/apache/flink-ml/pull/95#discussion_r877917528


##########
flink-ml-dist/pom.xml:
##########
@@ -19,14 +19,17 @@ under the License.
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
+    <properties>
+        <statefun-flink-core.version>3.2.0</statefun-flink-core.version>
+    </properties>

Review Comment:
   nit: Let's move this property to the parent pom.xml and reuse this property in flink-ml-dist and flink-ml-iteration pom files.
   
   Besides, `statefun.version` might be enough while more concise.



##########
flink-ml-lib/src/test/java/org/apache/flink/ml/clustering/OnlineKMeansTest.java:
##########
@@ -407,13 +415,15 @@ public void testSaveAndReload() throws Exception {
 
         transformAndOutputData(loadedOnlineModel);
 
-        miniCluster.submitJob(env.getStreamGraph().getJobGraph());
-        waitInitModelDataSetup();
+        final JobSubmissionResult jobSubmissionResult =
+                miniCluster.submitJob(env.getStreamGraph().getJobGraph()).get();
+        final JobID jobID = jobSubmissionResult.getJobID();

Review Comment:
   nit: I noticed that some tests uses 
   ```java
   final JobSubmissionResult jobSubmissionResult =
           miniCluster.submitJob(env.getStreamGraph().getJobGraph()).get();
   final JobID jobID = jobSubmissionResult.getJobID();
   ```
   while others use
   ```java
   final JobGraph jobGraph = env.getStreamGraph().getJobGraph();
   miniCluster.submitJob(jobGraph);
   final JobID jobID = jobGraph.getJobID();
   ```
   And I think it might be better to unify the practice across all implementations.



-- 
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@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org