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

getClosestGlobalNodes, findNode error fix #402

Merged
merged 1 commit into from
Jul 13, 2022
Merged

Conversation

tegefaulkes
Copy link
Contributor

@tegefaulkes tegefaulkes commented Jul 8, 2022

Description

This PR addresses...

#398 Was a problem with NodeConnectionManager.getClosestGlobalNodes throwing an error if the nodeGraph was empty. This shouldn't happen in usual operation unless said node is a seed node with no nodes in the network. Here it's not strictly an error since having no nodes in the graph just means we can't find any other nodes or the target node for that matter. getClosestGlobalNodes should've just returned undefined.

Issues Fixed

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@CMCDragonkai
Copy link
Member

@emmacasolin requires rebase on master. Also consult with @tegefaulkes and the situation and compare with what's going on AWS. Make sure you're able to run ./scripts/deploy-image.sh and ./scripts/deploy-service.sh.

@CMCDragonkai
Copy link
Member

Rename this PR more appropriately targeting what is being done here.

@CMCDragonkai
Copy link
Member

Regarding the worker manager fix. I want to put that off for moment. Can you separate the PRs... cause I the worker manager deserves more thought.

@tegefaulkes tegefaulkes changed the title Feature bug fixes getClosestGlobalNodes, findNode error fix Jul 12, 2022
@tegefaulkes
Copy link
Contributor Author

This PR was split into two. The workerManager changes are part of #406 now.

…deGraphEmptyDatabase` with empty network

`getClosestGlobalNodes` was throwing `ErrorNodeGraphEmptyDatabase` when it failed to get new nodes during the search process. Now it just returns undefined as expected.

Related #398
@tegefaulkes
Copy link
Contributor Author

This should be good to merge now. @CMCDragonkai

@tegefaulkes tegefaulkes merged commit 0894eab into staging Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development
Development

Successfully merging this pull request may close these issues.

Seed Nodes Restart after 1 hr due to ErrorNodeGraphEmptyDatabase
3 participants