You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/05/25 00:21:10 UTC

[GitHub] [spark] ArvinZheng opened a new pull request, #35484: [SPARK-38181][SS] Update comments in KafkaDataConsumer.scala

ArvinZheng opened a new pull request, #35484:
URL: https://github.com/apache/spark/pull/35484

   ### What changes were proposed in this pull request?
   Fixed some minor format issue in the code comments and rephrase some of them to make it more clear
   
   
   ### Why are the changes needed?
   Minor format correction and better readability
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   Not needed, no real code changes
   


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #35484: [SPARK-38181][SS][DOCS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #35484:
URL: https://github.com/apache/spark/pull/35484#issuecomment-1148140363

   Oh, also please enable GItHub Actions in your forked repository (https://github.com/ArvinZheng/spark). Apache Spark repository leverages PR author's GitHub Actions resources.


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] anishshri-db commented on pull request #35484: [SPARK-38181][SS][DOCS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
anishshri-db commented on PR #35484:
URL: https://github.com/apache/spark/pull/35484#issuecomment-1148138847

   Seems like the tests are failing though ? Maybe try merging back and re-trigger ?


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] github-actions[bot] closed pull request #35484: [SPARK-38181][SS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #35484: [SPARK-38181][SS] Update comments in KafkaDataConsumer.scala
URL: https://github.com/apache/spark/pull/35484


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] ArvinZheng commented on pull request #35484: [SPARK-38181][SS][DOCS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
ArvinZheng commented on PR #35484:
URL: https://github.com/apache/spark/pull/35484#issuecomment-1138160189

   @dongjoon-hyun @HeartSaVioR updated the branch and resolved the conflict, please help take a look when you get a chance, thanks in advance.


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HeartSaVioR commented on pull request #35484: [SPARK-38181][SS][DOCS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on PR #35484:
URL: https://github.com/apache/spark/pull/35484#issuecomment-1148596916

   Thanks! Merging to master.


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] anishshri-db commented on a diff in pull request #35484: [SPARK-38181][SS][DOCS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
anishshri-db commented on code in PR #35484:
URL: https://github.com/apache/spark/pull/35484#discussion_r890722033


##########
connector/kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/consumer/KafkaDataConsumer.scala:
##########
@@ -298,9 +296,10 @@ private[kafka010] class KafkaDataConsumer(
       s"requested $offset")
 
     // The following loop is basically for `failOnDataLoss = false`. When `failOnDataLoss` is
-    // `false`, first, we will try to fetch the record at `offset`. If no such record exists, then
-    // we will move to the next available offset within `[offset, untilOffset)` and retry.
-    // If `failOnDataLoss` is `true`, the loop body will be executed only once.
+    // `false`, we will try to fetch the record at `offset`, if the record does not exist, we will
+    // try to fetch next available record within [offset, untilOffset).
+    // If `failOnDataLoss` is `true`, the loop body will be executed only once, either return the
+    // record at `offset` or throw an exception when the record does not exist

Review Comment:
   Nit: Period at the end 



-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] github-actions[bot] commented on pull request #35484: [SPARK-38181][SS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #35484:
URL: https://github.com/apache/spark/pull/35484#issuecomment-1134031250

   We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
   If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] itholic commented on pull request #35484: [SPARK-38181][SS][DOCS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
itholic commented on PR #35484:
URL: https://github.com/apache/spark/pull/35484#issuecomment-1144442917

   Seems like the test still failing. Could you rebase?
   
   @HeartSaVioR FYI


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] ArvinZheng commented on a diff in pull request #35484: [SPARK-38181][SS][DOCS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
ArvinZheng commented on code in PR #35484:
URL: https://github.com/apache/spark/pull/35484#discussion_r890843318


##########
connector/kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/consumer/KafkaDataConsumer.scala:
##########
@@ -298,9 +296,10 @@ private[kafka010] class KafkaDataConsumer(
       s"requested $offset")
 
     // The following loop is basically for `failOnDataLoss = false`. When `failOnDataLoss` is
-    // `false`, first, we will try to fetch the record at `offset`. If no such record exists, then
-    // we will move to the next available offset within `[offset, untilOffset)` and retry.
-    // If `failOnDataLoss` is `true`, the loop body will be executed only once.
+    // `false`, we will try to fetch the record at `offset`, if the record does not exist, we will
+    // try to fetch next available record within [offset, untilOffset).
+    // If `failOnDataLoss` is `true`, the loop body will be executed only once, either return the
+    // record at `offset` or throw an exception when the record does not exist

Review Comment:
   thanks, 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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] ArvinZheng commented on pull request #35484: [SPARK-38181][SS][DOCS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
ArvinZheng commented on PR #35484:
URL: https://github.com/apache/spark/pull/35484#issuecomment-1144462037

   thanks @itholic for checking, just updated my branch, and yes, it's all doc/comment fixes 


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] ArvinZheng commented on pull request #35484: [SPARK-38181][SS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
ArvinZheng commented on PR #35484:
URL: https://github.com/apache/spark/pull/35484#issuecomment-1136182546

   @dongjoon-hyun , @HeartSaVioR , mind taking a look? just some simple changes for comments 


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #35484: [SPARK-38181][SS][DOCS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #35484:
URL: https://github.com/apache/spark/pull/35484#issuecomment-1148140028

   @ArvinZheng mind rebasing this PR? seems like something went wrong about finding GA actions in your fork.


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HeartSaVioR closed pull request #35484: [SPARK-38181][SS][DOCS] Update comments in KafkaDataConsumer.scala

Posted by GitBox <gi...@apache.org>.
HeartSaVioR closed pull request #35484: [SPARK-38181][SS][DOCS] Update comments in KafkaDataConsumer.scala
URL: https://github.com/apache/spark/pull/35484


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org