Skip to content

Make it possible to run VAST without user configs #1557

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

Merged

Conversation

dominiklohmann
Copy link
Member

@dominiklohmann dominiklohmann commented Apr 19, 2021

📔 Description

The new option --disable-default-config-dirs that may only be supplied on the command line disables the loading of user and system configuration, schema, and plugin directories. We use this option internally when generating VAST's man page and when running integration tests.

📝 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

🎯 Review Instructions

Commit-by-commit.

@dominiklohmann dominiklohmann added enhancement ✨ bug Incorrect behavior labels Apr 19, 2021
@dominiklohmann dominiklohmann requested a review from a team April 19, 2021 09:04
lava
lava previously approved these changes Apr 19, 2021
Copy link
Member

@lava lava left a comment

Choose a reason for hiding this comment

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

Having the option makes sense and is desired, but the current behavior was a bit surprising for me:

$ cat config.yaml 
vast:
  endpoint: "localhost:6000"

$ ./bin/vast --disable-configuration --config=config.yaml start
[13:56:51.138] loaded configuration file: "config.yaml"
[13:56:51.265] VAST node is listening on localhost:6000

Also I'd intuitively expect the environment variables to be ignored as well with this, since they're just another channel to pass configuration.

@lava lava dismissed their stale review April 19, 2021 12:00

Approval due to mis-click.

@dominiklohmann
Copy link
Member Author

dominiklohmann commented Apr 19, 2021

Also I'd intuitively expect the environment variables to be ignored as well with this, since they're just another channel to pass configuration.

The environment variables are just another workaround because of the same ordering issue we always have when loading plugins: Plugins can modify the command structure, and the command structure is required to parse the command line. Hence why the two variables must continue working, or we break integration tests for external plugins.

Similarly, --config still needs to work. Otherwise, the integration tests cannot run at all.

@dominiklohmann dominiklohmann force-pushed the story/ch24214/ch24589/run-without-configuration branch from 1ba55c9 to f3d2ca8 Compare April 19, 2021 12:20
@dominiklohmann dominiklohmann requested a review from lava April 19, 2021 12:20
@lava
Copy link
Member

lava commented Apr 19, 2021

Similarly, --config still needs to work. Otherwise, the integration tests cannot run at all.

Fine with me, but then we should adapt the option name & description accordingly. Currently the description says that it will also disable user configuration. Something like --disable-custom-directory-lookup?

@dominiklohmann dominiklohmann force-pushed the story/ch24214/ch24589/run-without-configuration branch from f3d2ca8 to d00f39a Compare April 19, 2021 15:41
@dominiklohmann
Copy link
Member Author

I've reworded to --bare-mode per our discussion in Slack.

Copy link
Member

@lava lava left a comment

Choose a reason for hiding this comment

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

Looking good now, module a few nits.

@dominiklohmann dominiklohmann force-pushed the story/ch24214/ch24589/run-without-configuration branch 2 times, most recently from ccfdd89 to 39602b8 Compare April 19, 2021 16:38
The new option `--bare-mode` that may only be supplied on the command
line disables the loading of user (and system) configuration, schema and
plugin directories.

This option is used internally when generating VAST's man-page and when
running integration tests.
@dominiklohmann dominiklohmann force-pushed the story/ch24214/ch24589/run-without-configuration branch from 39602b8 to 443331b Compare April 20, 2021 08:03
@dominiklohmann
Copy link
Member Author

Renamed again to --disable-default-config-dirs after further discussion in Slack.

@dominiklohmann dominiklohmann merged commit f0d482b into master Apr 20, 2021
@dominiklohmann dominiklohmann deleted the story/ch24214/ch24589/run-without-configuration branch April 20, 2021 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants