-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add plugin-specific setup sub-directory #178
Conversation
I think the only missing change is docs-related. Going to do that soon, but opening for reviews already |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ecdsa_private_key_store_path: /near-sffl/config/keys/ecdsa.json | ||
bls_private_key_store_path: /near-sffl/config/keys/bls.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this one, but shouldn't these paths be relative (./
) rather than absolute (/
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, these paths are from the container's point of view - so considering the shared plugin
setup dir is mapped to /near-sffl
, then those can be absolute like that
# manually register the operator - not recommended | ||
register_operator_on_startup: true | ||
# manually register the operator. | ||
register_operator_on_startup: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that parameter planned to be used - ever to be true? It is quite an implicit one. Would be nice to get rid of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to keep it, as it's convenient for a user that might want to use it and also for testing
It's not that nice having the plugin together with the operator
setup
. The most notable situation is if an operator doesn't want to use theRegisterOnStartup
setting to preserve their private key. In this case, it's not practical to change the config or have two copies of the repo.This PR adds its own
setup/
sub-directory to the operator plugin, with a separate config file,.env
and so on.