Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Nc 2107 permissioning config toml file #643

Merged
merged 51 commits into from
Jan 29, 2019
Merged

Nc 2107 permissioning config toml file #643

merged 51 commits into from
Jan 29, 2019

Conversation

macfarla
Copy link
Contributor

@macfarla macfarla commented Jan 23, 2019

PR description

Parse TOML config file for permissions config options
Also added CLI options
--permissions-nodes-enabled
--permissions-accounts-enabled

Fixed Issue(s)

NC-2107

@macfarla macfarla added the work in progress Work on this pull request is ongoing label Jan 23, 2019
@macfarla macfarla removed the work in progress Work on this pull request is ongoing label Jan 25, 2019
if (permissioningConfigFile.exists()) {
return permissioningConfigFile;
} else {
LOG.error("Unable to load permissioning config file from location {}.", filename);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be more explicit that the file doesn't exist at the specified location? I feel like this could be interpreted as "something went wrong", not "it wasn't there."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@mark-terry mark-terry left a comment

Choose a reason for hiding this comment

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

LGTM...

@macfarla macfarla merged commit 7660e99 into PegaSysEng:master Jan 29, 2019
@macfarla macfarla deleted the nc-2107-permissioning-config-toml-file branch January 29, 2019 11:57
vinistevam pushed a commit to vinistevam/pantheon that referenced this pull request Jan 29, 2019
* create permissioning config from toml file

* added CLI options for separately enabling node and account permissioning

* moved check for bootnodes on whitelist out of CLI
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.

2 participants