You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "hdulay (via GitHub)" <gi...@apache.org> on 2023/11/11 02:57:41 UTC

[PR] upgrading flink dep to 1.17.1 [pinot]

hdulay opened a new pull request, #11991:
URL: https://github.com/apache/pinot/pull/11991

   In testing the Flink-Pinot sink connector, the version of the Kafka source connector was getting a NPE. By upgrading the Flink version to `1.17.1`, I was able to deploy a Flink job with Kafka source and Pinot sink.
   
   `dependencies`
   


-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Upgrading Flink dep to 1.17.1 [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11991:
URL: https://github.com/apache/pinot/pull/11991#discussion_r1397966821


##########
pinot-connectors/pinot-flink-connector/pom.xml:
##########
@@ -51,7 +51,15 @@
     </dependency>
     <dependency>
       <groupId>org.apache.flink</groupId>
-      <artifactId>flink-streaming-java_${scala.compat.version}</artifactId>
+      <artifactId>flink-streaming-java</artifactId>
+      <version>${flink.version}</version>

Review Comment:
   @walterddr It is already defined in the root pom, and we are overriding it here



-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Upgrading Flink dep to 1.17.1 [pinot]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11991:
URL: https://github.com/apache/pinot/pull/11991#issuecomment-1810983688

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11991?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   All modified and coverable lines are covered by tests :white_check_mark:
   > Comparison is base [(`b38bd9f`)](https://app.codecov.io/gh/apache/pinot/commit/b38bd9fb34aa06e53e11f540c1315306d023a9c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.63% compared to head [(`54c6afa`)](https://app.codecov.io/gh/apache/pinot/pull/11991?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 35.06%.
   > Report is 9 commits behind head on master.
   
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #11991       +/-   ##
   =============================================
   - Coverage     61.63%   35.06%   -26.58%     
   - Complexity      207      934      +727     
   =============================================
     Files          2385     2309       -76     
     Lines        129214   125468     -3746     
     Branches      20003    19445      -558     
   =============================================
   - Hits          79640    43991    -35649     
   - Misses        43773    78370    +34597     
   + Partials       5801     3107     -2694     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <ø> (ø)` | |
   | [integration](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <ø> (ø)` | |
   | [integration1](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <ø> (ø)` | |
   | [integration2](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | |
   | [java-11](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `35.00% <ø> (-0.01%)` | :arrow_down: |
   | [java-21](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.94% <ø> (-26.57%)` | :arrow_down: |
   | [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `35.02% <ø> (-26.58%)` | :arrow_down: |
   | [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.92% <ø> (-26.58%)` | :arrow_down: |
   | [temurin](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `35.06% <ø> (-26.58%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.97% <ø> (-14.66%)` | :arrow_down: |
   | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.97% <ø> (+0.02%)` | :arrow_up: |
   | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/11991/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/11991?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Upgrading Flink dep to 1.17.1 [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11991:
URL: https://github.com/apache/pinot/pull/11991#discussion_r1393013044


##########
pinot-connectors/pinot-flink-connector/pom.xml:
##########
@@ -51,7 +51,15 @@
     </dependency>
     <dependency>
       <groupId>org.apache.flink</groupId>
-      <artifactId>flink-streaming-java_${scala.compat.version}</artifactId>
+      <artifactId>flink-streaming-java</artifactId>
+      <version>${flink.version}</version>

Review Comment:
   Please modify the version within the root pom file, and only reference them here



-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Upgrading Flink dep to 1.17.1 [pinot]

Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #11991:
URL: https://github.com/apache/pinot/pull/11991#discussion_r1396528468


##########
pinot-connectors/pinot-flink-connector/pom.xml:
##########
@@ -51,7 +51,15 @@
     </dependency>
     <dependency>
       <groupId>org.apache.flink</groupId>
-      <artifactId>flink-streaming-java_${scala.compat.version}</artifactId>
+      <artifactId>flink-streaming-java</artifactId>
+      <version>${flink.version}</version>

Review Comment:
   IMO if flink is only being used inside this plugin it is ok to keep the versioning here. only when it is being used by a plugin and a core module (jackson for example) should the version be defined in root pom



-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Upgrading Flink dep to 1.17.1 [pinot]

Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #11991:
URL: https://github.com/apache/pinot/pull/11991#discussion_r1398039884


##########
pinot-connectors/pinot-flink-connector/pom.xml:
##########
@@ -51,7 +51,15 @@
     </dependency>
     <dependency>
       <groupId>org.apache.flink</groupId>
-      <artifactId>flink-streaming-java_${scala.compat.version}</artifactId>
+      <artifactId>flink-streaming-java</artifactId>
+      <version>${flink.version}</version>

Review Comment:
   oh. ok. didn't know we moved it out. 



-- 
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: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org