[ETCM-121] Refactor configs #681

ntallar · 2020-09-18T18:11:15Z

Description

Different changes with the objectives of:

Naming our current testnet to internal
Allow starting Mantis on different environments without having to comment/uncomment lines of configuration files

Important Changes Introduced

Removes private testnet configurations as there is no use case for them yet
Adds testnet-internal configurations
Lots of renamings (i.e etc.json -> etc-genesis.json, etc.conf -> etc-chain.conf)

Up-to-discussion

Removing "user friendly" configurations as there haven't proven to be very used or even friendly (the other changes are compatible with this if we want to have them)
Should we enable HTTPS on the application.conf instead?

Testing

Sbt dist and sbt run should work as always with trying out all the different environments (the packaged binaries now list all the environments currently supported)

src/universal/bin/eckeygen

src/universal/bin/faucet-server

src/universal/bin/mantis-mordor

mirkoAlic · 2020-09-18T18:48:17Z

I think is the way to go =) great job!

src/universal/conf/testmode.conf

mirkoAlic · 2020-09-18T19:01:50Z

please update the README -> https://github.com/input-output-hk/mantis#command-line-version

mmrozek · 2020-09-21T08:17:48Z

src/main/resources/chains/etc-chain.conf

@@ -0,0 +1 @@
+{ include "base-chain.conf"}


Do we need base-chain?

It was created as etc and eth networks share a lot of common configuration

Yes, but base == etc. In my opinion, we don't need it

Hmm I'm also a bit on Michal's side in can be a bit confusing

In my opinion I'd just replicate the duplicated configuration in both etc-chain.conf and eth-chain.conf, just to have to check a single file to see all the configurations of a chain. This kinds of files will rarely be touched so I don't see that being too much of a problem

Go for it then!

Just updated them! Please check that I haven't screwed up in the middle

I noticed some ropsten configuration wasn't properly configured but I'd leave it as is to not fall down in that rabbit hole, if we ever start providing official support for it (seems to be the intent eventually) we'll fix it then

src/universal/conf/app.conf

kapke · 2020-09-21T12:53:29Z

src/universal/conf/app.conf

+      http {
+        mode = "https"
+
+        certificate-keystore-path = "conf/mantis.jks"


Should HTTPS be pre-configured with these values or disabled by default so user can configure it on its own?

Imo for now I'd just disable it and have the user configure it on its own, we can later add it for development's purposes or provide some guide on to how the user should configure it

kapke · 2020-09-21T12:59:08Z

Misclick 😅
Regarding launcher scripts - I think we should add *.bat versions of them as well. And that leads to another conclusion - should we maybe replace them with command-line argument, so we don't need to create such number of files manually?

mmrozek · 2020-09-21T13:13:50Z

@kapke, according to your last comment, maybe the generic script will be enough?
sth like:

#!/bin/bash

chain="$1"
if [ -z "$chain"]
then
  echo "You need to choose chain"
else
  shift
  ./bin/mantis -Dconfig.file=./conf/"$chain".conf "$@"
fi

kapke · 2020-09-21T13:36:38Z

@mmrozek I'm not sure. There are 2 issues in that approach:

how to name generic mantis launcher so it doesn't interfere with that one
there's need to write and maintain *.bat version of it

ntallar · 2020-09-21T15:35:06Z

Maybe it's better than nothing to have the script that Michal mentioned? (maybe we could name it mantis-app or mantis-launcher). We could add the .bat version in a separate PR.

The other alternative is just removing this script, is it used by any for now?

…auncher; removed unnecessary configurations from testnet internal; removed https from app.conf

mmrozek

lgtm!

kapke · 2020-09-22T10:50:39Z

src/universal/bin/mantis-launcher

@@ -0,0 +1,10 @@
+#!/bin/bash


It doesn't have execute permission

kapke · 2020-09-22T14:40:24Z

src/main/resources/chains/eth-chain.conf

+
+  # Custom genesis JSON file path
+  # null value indicates using default genesis definition that matches the main network
+  custom-genesis-file = null


minor: should we maybe get rid of optional genesis path? (separate task for Piotr or Adiran maybe?)

Totally! Here's the task for that: https://jira.iohk.io/browse/ETCM-128

kapke

LGTM!

ntallar force-pushed the etcm-121-refactor-configs branch from 7329ff7 to b013bc7 Compare September 18, 2020 18:12

mirkoAlic reviewed Sep 18, 2020

View reviewed changes

src/universal/bin/eckeygen Outdated Show resolved Hide resolved

src/universal/bin/faucet-server Outdated Show resolved Hide resolved

src/universal/bin/mantis-mordor Outdated Show resolved Hide resolved

ntallar force-pushed the etcm-121-refactor-configs branch 2 times, most recently from baedddd to 3d2893b Compare September 18, 2020 18:45

mirkoAlic reviewed Sep 18, 2020

View reviewed changes

src/universal/conf/testmode.conf Show resolved Hide resolved

[ETCM-121] Refactor configs

8b03317

ntallar force-pushed the etcm-121-refactor-configs branch from 3d2893b to 8b03317 Compare September 18, 2020 18:54

ntallar marked this pull request as ready for review September 18, 2020 18:54

ntallar changed the title ~~[ETCM-121] Refactor configs (WIP proposal)~~ [ETCM-121] Refactor configs Sep 18, 2020

ntallar requested review from kapke, jmendiola222 and mmrozek September 18, 2020 18:54

ntallar mentioned this pull request Sep 18, 2020

[ETCM-43][ETCM-46] Add opt-out field #675

Merged

3 tasks

ntallar added the BREAKS CONFIG Affects the default configuration label Sep 18, 2020

mmrozek reviewed Sep 21, 2020

View reviewed changes

kapke reviewed Sep 21, 2020

View reviewed changes

kapke closed this Sep 21, 2020

kapke reopened this Sep 21, 2020

[ETCM-121] Replaced bash script for every chain for a single mantis-l…

e02a017

…auncher; removed unnecessary configurations from testnet internal; removed https from app.conf

mmrozek approved these changes Sep 22, 2020

View reviewed changes

kapke reviewed Sep 22, 2020

View reviewed changes

src/universal/bin/mantis-launcher

@@ -0,0 +1,10 @@

#!/bin/bash

Copy link

Contributor

kapke Sep 22, 2020

Choose a reason for hiding this comment

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

It doesn't have execute permission

Nicolas Tallar added 2 commits September 22, 2020 10:06

[ETCM-121] Removed base-chain.conf

b0f9a11

[ETCM-121] Make mantis-launcher executable

c6ae3e7

ntallar requested a review from kapke September 22, 2020 13:20

kapke reviewed Sep 22, 2020

View reviewed changes

kapke approved these changes Sep 23, 2020

View reviewed changes

Nicolas Tallar added 2 commits September 23, 2020 09:15

[ETCM-121] Merge with develop

83dacb1

[ETCM-121] Merged with develop

4b41ac9

ntallar merged commit fcdc21c into develop Sep 24, 2020

ntallar deleted the etcm-121-refactor-configs branch September 24, 2020 11:25

[ETCM-121] Refactor configs #681

[ETCM-121] Refactor configs #681

Uh oh!

Conversation

ntallar commented Sep 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Important Changes Introduced

Up-to-discussion

Testing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mirkoAlic commented Sep 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mirkoAlic commented Sep 18, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kapke commented Sep 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmrozek commented Sep 21, 2020

Uh oh!

kapke commented Sep 21, 2020

Uh oh!

ntallar commented Sep 21, 2020

Uh oh!

mmrozek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kapke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ntallar commented Sep 18, 2020 •

edited

Loading

mirkoAlic commented Sep 18, 2020 •

edited

Loading

kapke commented Sep 21, 2020 •

edited

Loading