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

Simplify local boot and enable multi node #1751

Merged
merged 8 commits into from
Dec 18, 2018

Conversation

sabau
Copy link
Contributor

@sabau sabau commented Dec 14, 2018

Signed-off-by: Karoly Albert Szabo szabo.karoly.a@gmail.com

Closes #1661

Description:

  • reduce side effects:
    • at configuration level: I don't have to know that informations are being placed in my home folder, everything is local to the project and the network name I've chosen
    • at code level helpers are functional and don't rely on global status (first of a series of this kind)
    • You can import helpers without side effect (and test them lately 🎉)
    • if I clone several times the project I can expect them to run indipendently without iteracting on their mutual configurations (of course not at the same time :) )
  • taking out the functions to work with multiple nodes
  • on the road to reduce the code amount in the boot process for local node
  • enabling build + running with one command for multiple nodes
  • reduce hardcoded parameters, instead pass them through functions

❤️ Thank you!


  • Added entries in CHANGELOG.md with issue # and GitHub username
  • Reviewed Files changed in the github PR explorer

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>
@sabau sabau requested review from faboweb and nylira as code owners December 14, 2018 17:16
@sabau sabau changed the title [WIP] taking out the functions to work with multiple nodes [WIP] Simplify local boot and enable multi node Dec 14, 2018
@codecov
Copy link

codecov bot commented Dec 14, 2018

Codecov Report

Merging #1751 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           develop   #1751   +/-   ##
=======================================
  Coverage     97.7%   97.7%           
=======================================
  Files          102     102           
  Lines         2096    2096           
  Branches        93      93           
=======================================
  Hits          2048    2048           
  Misses          39      39           
  Partials         9       9
Impacted Files Coverage Δ
app/src/renderer/components/common/ToolBar.vue
app/src/renderer/components/common/VmToolBar.vue 100% <0%> (ø)

@codecov
Copy link

codecov bot commented Dec 14, 2018

Codecov Report

Merging #1751 into develop will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #1751      +/-   ##
===========================================
+ Coverage     97.7%   97.71%   +<.01%     
===========================================
  Files          102      102              
  Lines         2096     2097       +1     
  Branches        93       93              
===========================================
+ Hits          2048     2049       +1     
  Misses          39       39              
  Partials         9        9
Impacted Files Coverage Δ
app/src/root.js 100% <100%> (ø) ⬆️

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>
tasks/testnet.js Outdated Show resolved Hide resolved
@faboweb
Copy link
Collaborator

faboweb commented Dec 17, 2018

Awesome :)

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>
…counts of the validators

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>
…odes

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>
Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>
numberNodes
)
await startNodes(nodes, mainAccountSignInfo, network)
fs.copySync(join(nodes[1].home, `config`), cliHomePrefix)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This mimic the behaviour of electron start, I tried hard not to put those three lines here and rely on that hook but I couldn't find a nice way of doing it. Any idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From copySync to WriteFileSync should happen at boot in my mind, instead I'm forcing it to happen at network creation time (I didn't wanted to touch also that process right now, to keep this PR small)

Copy link
Collaborator

Choose a reason for hiding this comment

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

boot process will die anyway ;)

@sabau sabau changed the title [WIP] Simplify local boot and enable multi node Simplify local boot and enable multi node Dec 18, 2018
}

// nodes[0] is a placeholder just to be aligned with the enumeration used in gaia
// TODO: next PR refactor also gaia to simplify numeration
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@faboweb faboweb merged commit 40f1f05 into develop Dec 18, 2018
@faboweb faboweb deleted the sabau/1661-multinode-local-testnet branch December 18, 2018 15:54
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