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

refactor codebase with urfave/cli #120

Merged
merged 28 commits into from
Aug 6, 2021
Merged

refactor codebase with urfave/cli #120

merged 28 commits into from
Aug 6, 2021

Conversation

tiero
Copy link
Member

@tiero tiero commented Aug 3, 2021

Overview

This PR starts the groundwork for #113 simplifying the codebase: the main takeway is that we do not need to load compose files and nodes' configuration files out of band (ie. installer script) but we ship them inside the compiled binary using the new embed package introduced in the standard library from Go 1.6

This does not introduce or makes any breaking changes that can be noticed by end users of any means

Notable changes

  • To change datadir will be used an env var NIGIRI_DATADIR instead of a global flag, which introduces a chicken-egg problem, since in the init of the main.go we check/provision the resources automatically, without using an installer script.
  • a flag --ci has been added which will allow to run in headless mode (ie. without Esplora frontend) useful in continuos integration pipelines.
  • an update command has been added to pull new docker images from the Github Container Registry

TODOs

  • add --ci flag to run in headless mode
  • change datadir via NIGIRI_DATADIR
  • add update command to pull down new images
  • reintroduces testing suite
  • Update Makefile
  • Update scripts
  • update GH actions

@tiero tiero requested a review from altafan August 3, 2021 15:44
@tiero tiero changed the title refactor codebase with urfave/cli WIP refactor codebase with urfave/cli Aug 3, 2021
@tiero tiero changed the title WIP refactor codebase with urfave/cli refactor codebase with urfave/cli Aug 4, 2021
@tiero
Copy link
Member Author

tiero commented Aug 4, 2021

Still not satisfied about hot properly test it. Provably the best way should be an e2e building the binary and run it from go code.

tiero and others added 2 commits August 4, 2021 13:48
Containers started through this docker compose file will be in this isolated network.
Note: the minimum version for the docker compose file format is 3.5 which introduces `name` for networks. This file format requires a docker engine version of 17.12.0+.
Copy link
Collaborator

@altafan altafan left a comment

Choose a reason for hiding this comment

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

This refactor was soooo much needed! thanks for the effort.

I have a question: is this intended to let one start multiple nigiris up on the same machine? If so the exposed ports of the services of the compose file must not be hardcoded.

cmd/nigiri/resources/docker-compose-regtest-liquid.yml Outdated Show resolved Hide resolved
internal/config/config.go Outdated Show resolved Hide resolved
internal/config/config.go Outdated Show resolved Hide resolved
pkg/state/state.go Outdated Show resolved Hide resolved
test/start_stop_test.go Outdated Show resolved Hide resolved
@tiero
Copy link
Member Author

tiero commented Aug 6, 2021

is this intended to let one start multiple nigiris up on the same machine

At that point someone can use different data directory and manually tweak the compose that is moved there.

# start first
nigiri start --liquid
# only provision datadir
nigiri --datadir=mydata version
# tweak compose in ./mydata
# nigiri --datadir=mydata start --liquid

@tiero tiero merged commit 70e22ba into vulpemventures:master Aug 6, 2021
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.

3 participants