Skip to content
This repository has been archived by the owner on Jun 30, 2021. It is now read-only.

Prepare commands and docs for v1.1.0 release #733

Merged
merged 42 commits into from Jan 28, 2019
Merged

Prepare commands and docs for v1.1.0 release #733

merged 42 commits into from Jan 28, 2019

Conversation

unnawut
Copy link
Contributor

@unnawut unnawut commented Jan 24, 2019

Issue/Task Number: #684

Overview

This PR updates release tasks and docs in preparation for v1.1.0 release.

Changes

  • Remove asdf-specific .tool-versions
  • Split EWallet.ReleaseTasks into multiple modules separated by the main task
  • Add bin/ewallet config --migrate support
  • Add mix omg.config that has feature-parity with bin/wallet config
  • Add a generic upgrade docs
  • Add a v1.1.0 upgrade docs

Implementation Details

Turns out v1.0.0 -> v1.1.0 is quite a smooth ride since breaking changes were already crammed within v1.0.0.

The biggest change is the distillery release which renders mix commands unavailable on production builds, while itself is quite cumbersome to use in development environment.

This PR mainly adds documentations regarding upgrades, as well as attempting towards feature-parity (but not yet) between mix omg.* commands for dev environment and bin/ewallet for prod environment.

Usage

See upgrade instructions under Upgrade heading on the main README.md.

Commands:

Action Dev Prod
Print config instructions mix omg.config
or mix omg.config -h
bin/ewallet config
or bin/ewallet config -h
Set a configuration mix omg.config key value bin/ewallet config key value
Migrate configurations mix omg.config --migrate bin/ewallet config --migrate

Impact

Nothing specific for this PR, but the command bin/ewallet config --migrate must be run during the upgrade from v1.0.0 to v1.1.0.

@ghost ghost assigned unnawut Jan 24, 2019
@ghost ghost added the s2/wip 🚧 label Jan 24, 2019
# Conflicts:
#	rel/commands/config.sh
@@ -41,6 +41,10 @@ For other platforms or for a more advanced setup, see also manual installation b

- [Bare metal installation](docs/setup/bare_metal.md)

### Upgrade

Upgrading to a newer version? See [Upgrading the eWallet Server](docs/setup/upgrading).
Copy link

Choose a reason for hiding this comment

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

Maybe the eWallet server application?

Copy link
Contributor

Choose a reason for hiding this comment

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

I try to call it "OmiseGO eWallet Server" everywhere if possible.

Copy link
Contributor Author

@unnawut unnawut Jan 25, 2019

Choose a reason for hiding this comment

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

and then there's the Suite...

Hence I'm going to create a new page called Glossary to discuss & pin this down for good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@T-Dnzt @sirn I've just added a glossary section. Please check a98169a.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@unnawut
Copy link
Contributor Author

unnawut commented Jan 25, 2019

@sirn I splitted ReleaseTasks into smaller ReleaseTasks.* modules.

And made a bit of changes to rel/commands/config.sh and rel/commands/seed.sh to do the bin/ewallet config --migrate.

Can you have a look?

Copy link
Contributor

@sirn sirn left a comment

Choose a reason for hiding this comment

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

Few comments here and there. Also can you run ShellCheck against all the shell scripts?

.gitignore Outdated
@@ -24,5 +24,8 @@ erl_crash.dump
.elixir_ls
Thumbs.db

# asdf
.tool-versions
Copy link
Contributor

Choose a reason for hiding this comment

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

Whyyyyy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eh. Thought that this is supposed to be user-specific.

But using this to semi-enforce dev users to use the same tool versions is fine I guess. Removing.

Copy link
Contributor

Choose a reason for hiding this comment

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

This thing actually saved my life quite few times already since I run editor outside of my Docker VM which has different Elixir version than the one in omisego-images/docker-ewallet (and .tool-versions yell at me to install 1.6.5 which is nice…)

# Mix is not available with distillery releases.
# Link: https://github.com/elixir-lang/elixir/blob/v1.6.5/lib/mix/lib/mix/shell/io.ex#L54
answer = IO.gets(message <> " [Yn] ")
is_binary(answer) and String.trim(answer) in ["" | @yes_inputs]
Copy link
Contributor

Choose a reason for hiding this comment

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

Might worth to use EWallet.Helper.to_boolean instead as that one handle plenty of… weird cases.


def run(opts \\ []) do
# Make sure the settings seed has been run
_ = Seed.run_settings_no_stop()
Copy link
Contributor

Choose a reason for hiding this comment

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

Would prefer this to re-implement the seed apps initialization because it serves different purpose na.


KEY="$(printf "%s" "$1" | base64)"; shift
VALUE="$(printf "%s" "$1" | base64)"; shift
if [ $ACTION = migrate ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the better way to implement this is to do:

run_migrate() {
  # exec ...
}

run_set() {
  # exec...
}

case "$ACTION" in
  set ) run_set;;
  migrate ) run_migrate;;
  * )
    print_usage
    exit 1
esac

I try to use exit 1 for hard error (e.g. wrong arguments, etc.), and exit 2 for informative errors (printing usage, etc.) because it's pretty common to check for exit 1 for detecting errors.

KEY="$(printf "%s" "$1" | base64)"; shift
VALUE="$(printf "%s" "$1" | base64)"; shift
if [ $ACTION = migrate ]; then
$RELEASE_ROOT_DIR/bin/ewallet command Elixir.EWallet.ReleaseTasks.Seed run_settings &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need && here? If you want to detect that the command ran successfully, better use:

if ! $RELEASE_ROOT_DIR/bin/ewallet command Elixir.EWallet.ReleaseTasks.Seed run_settings; then
  printf "The error occurred during seeding settings data.\\n"
  exit 1
fi


exec "$RELEASE_ROOT_DIR/bin/ewallet" command Elixir.EWallet.ReleaseTasks config_base64 "$KEY" "$VALUE"
if [ "$ASK_CONFIRM" = true ]; then
exec "$RELEASE_ROOT_DIR/bin/ewallet" command Elixir.EWallet.ReleaseTasks.ConfigMigration run_ask_confirm
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to have only one exec at the end of execution because it can be confusing since exec is essentially the end of shell script. So here better do:

MIGRATION_FN = "run_ask_confirm"

if [ "$ASK_CONFIRM" = true ]; then
  MIGRATION_FN = "run_skip_confirm"
fi

exec …

@unnawut
Copy link
Contributor Author

unnawut commented Jan 25, 2019

@sirn Updated. Please check again.

@unnawut unnawut added this to the v1.1 milestone Jan 25, 2019
@unnawut unnawut merged commit 96d847f into v1.1 Jan 28, 2019
@ghost ghost removed the s3/review 👀 label Jan 28, 2019
@unnawut unnawut deleted the 684-upgrade-path branch January 28, 2019 04:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants