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

feat: add service options #691

Closed
wants to merge 1 commit into from
Closed

feat: add service options #691

wants to merge 1 commit into from

Conversation

reliveyy
Copy link
Collaborator

@reliveyy reliveyy commented Aug 26, 2020

The PR removes .conf file usage from all images. And I also added serveral options --<service>.options to let the end users (include developers) to customize service launching command-line arguments.

Resolves #405

How to test

bash xud.sh -b service-options --geth.options="--cache 2048"

The custom options were applied in two ways

  1. Inject into an environment variable like "XUD_OPTS"
  2. Append by "$@"

Known issues

  • A separated PR for removing xud.conf (see chore: remove xud.conf #722)
  • A separated PR for removing lnd.conf (see chore: remove lnd.conf #723)
  • Move options from service images' entrypoint.sh into utils
  • A separated PR for editing service options without building new images. (TBD) If the option specified in --<service>.options exists in default settings and some program may not allow overriding previous options, then it could throw an error

@reliveyy reliveyy requested a review from raladev August 26, 2020 09:21
@reliveyy reliveyy self-assigned this Aug 26, 2020
@reliveyy reliveyy marked this pull request as draft August 26, 2020 09:23
@reliveyy reliveyy force-pushed the service-options branch 2 times, most recently from 6ca21c6 to f2656c4 Compare September 2, 2020 13:08
@reliveyy reliveyy marked this pull request as ready for review September 14, 2020 11:44
@kilrau
Copy link
Contributor

kilrau commented Sep 14, 2020

Looking at this I have the following concerns as discussed with @reliveyy :

  • one needs to build a new image to apply a new option
  • one can only append options
  • logic looks complicated and somewhat error prone

Especially the first one is not acceptable for users in my opinion.

What we really want is: the user to be able to modify an option via mainnet.conf and via xud.conf/lnd.conf and xud-docker to apply these options. In order to allow manipulating xud-docker's mainnet.conf and e.g. lndbtc's lnd.conf both, which have an intersecting set of options, like mode = light equals bitcoin.node = neutrino all we should need to do is: apply mainnet.conf options as cli parameters (which by default supersede config files) and the rest is read from the e.g. lnd.conf file. Not sure if made myself clear :/

@reliveyy reliveyy marked this pull request as draft September 15, 2020 02:48
@reliveyy reliveyy closed this Sep 29, 2020
@reliveyy reliveyy deleted the service-options branch October 13, 2020 08:08
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.

Don't use conf files
2 participants