Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: chunkify handleFindNode #232

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Jan 4, 2023

Motivation

  • In handleFindNode, we keep adding value to the resulting array then do a slice() to make sure the resulting nodes contain 16 items at most, we should push() then check() instead
  • The embeded chunkify work has duplicate code and not testable

Description

  • Add chunkify util

part of #201

@twoeths twoeths requested a review from a team as a code owner January 4, 2023 08:30
if (nodes.length === 0) {
log("Sending empty NODES response to %o", nodeAddr);
try {
this.sessionService.sendResponse(nodeAddr, createNodesMessage(id, 1, nodes));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this PR we no longer send a NODES response if we've found no nodes, is that intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when there is no nodes, the chunkify function returns [[]] (see the unit test), in that case we still send 1 response with total = 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants