You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "splett2 (via GitHub)" <gi...@apache.org> on 2023/04/05 14:34:22 UTC

[GitHub] [kafka] splett2 opened a new pull request, #13501: MINOR: Use `exists` instead of return in DelayedFetch

splett2 opened a new pull request, #13501:
URL: https://github.com/apache/kafka/pull/13501

   Using a return statement in a scala lambda is somewhat non-idiomatic. We can use an `exists` instead of a `foreach` and still short-circuit the loop on a `true`.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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: jira-unsubscribe@kafka.apache.org

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


[GitHub] [kafka] splett2 commented on a diff in pull request #13501: MINOR: Use `exists` instead of return in DelayedFetch

Posted by "splett2 (via GitHub)" <gi...@apache.org>.
splett2 commented on code in PR #13501:
URL: https://github.com/apache/kafka/pull/13501#discussion_r1158871931


##########
core/src/main/scala/kafka/server/DelayedFetch.scala:
##########
@@ -93,14 +93,14 @@ class DelayedFetch(
               if (endOffset.onOlderSegment(fetchOffset)) {
                 // Case F, this can happen when the new fetch operation is on a truncated leader
                 debug(s"Satisfying fetch $this since it is fetching later segments of partition $topicIdPartition.")
-                return forceComplete()
+                forceComplete()

Review Comment:
   no, good point. Let me take a look and see if I can address this.



-- 
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: jira-unsubscribe@kafka.apache.org

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


[GitHub] [kafka] jolshan commented on a diff in pull request #13501: MINOR: Use `exists` instead of return in DelayedFetch

Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on code in PR #13501:
URL: https://github.com/apache/kafka/pull/13501#discussion_r1158802468


##########
core/src/main/scala/kafka/server/DelayedFetch.scala:
##########
@@ -93,14 +93,14 @@ class DelayedFetch(
               if (endOffset.onOlderSegment(fetchOffset)) {
                 // Case F, this can happen when the new fetch operation is on a truncated leader
                 debug(s"Satisfying fetch $this since it is fetching later segments of partition $topicIdPartition.")
-                return forceComplete()
+                forceComplete()

Review Comment:
   Does this have the same semantics if forceComplete returns false?



-- 
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: jira-unsubscribe@kafka.apache.org

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


[GitHub] [kafka] github-actions[bot] commented on pull request #13501: MINOR: Use `exists` instead of return in DelayedFetch

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13501:
URL: https://github.com/apache/kafka/pull/13501#issuecomment-1620971270

   This PR is being marked as stale since it has not had any activity in 90 days. If you would like to keep this PR alive, please ask a committer for review. If the PR has  merge conflicts, please update it with the latest from trunk (or appropriate release branch) <p> If this PR is no longer valid or desired, please feel free to close it. If no activity occurrs in the next 30 days, it will be automatically closed.


-- 
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: jira-unsubscribe@kafka.apache.org

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


[GitHub] [kafka] splett2 closed pull request #13501: MINOR: Use `exists` instead of return in DelayedFetch

Posted by "splett2 (via GitHub)" <gi...@apache.org>.
splett2 closed pull request #13501: MINOR: Use `exists` instead of return in DelayedFetch
URL: https://github.com/apache/kafka/pull/13501


-- 
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: jira-unsubscribe@kafka.apache.org

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