-
Notifications
You must be signed in to change notification settings - Fork 61
updating autoReload feature #345
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Angelina <[email protected]>
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.
Thanks @aceppaluni a few comments, the autoReload was put in the wrong place
Have you tested this by changing something the config in the runtime folder of a running stack and seeing it reload it?
initCmd.PersistentFlags().StringArrayVar(&initOptions.NodeNames, "node-name", []string{}, "Node name") | ||
initCmd.PersistentFlags().BoolVar(&initOptions.RemoteNodeDeploy, "remote-node-deploy", false, "Enable or disable deployment of FireFly contracts on remote nodes") | ||
initCmd.PersistentFlags().StringToStringVar(&initOptions.EnvironmentVars, "environment-vars", map[string]string{}, "Common environment variables to set on all containers in FireFly stack") | ||
initCmd.Flags().BoolVar(&initOptions.EnableAutoReload, "auto-reload", true, "Enable config auto reload in generated FireFly configs") |
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.
initCmd.Flags().BoolVar(&initOptions.EnableAutoReload, "auto-reload", true, "Enable config auto reload in generated FireFly configs") | |
initCmd.Flags().BoolVar(&initOptions.EnableAutoReload, "auto-reload", true, "Enable config auto reload in FireFly Core") |
Event *EventConfig `yaml:"event,omitempty"` | ||
Plugins *Plugins `yaml:"plugins"` | ||
Namespaces *NamespacesConfig `yaml:"namespaces"` | ||
AutoReload bool `json:"autoReload" yaml:"autoReload"` |
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.
You only need the YAML one here
AutoReload bool `json:"autoReload" yaml:"autoReload"` | |
AutoReload bool `yaml:"autoReload"` |
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 believe this is under config so config.autoReload
see https://github.com/hyperledger/firefly/blob/f67a468378abc7f512231ccda7db626845e0f1ec/internal/coreconfig/coreconfig.go#L130
This PR updates autoReload