[FIXED] Config Reload: JetStream command line override ignored#6609
[FIXED] Config Reload: JetStream command line override ignored#6609derekcollison merged 2 commits intomainfrom
Conversation
conf/parse.go
Outdated
| } | ||
| } | ||
|
|
||
| func DisableConfigurationDigest(disable bool) { |
There was a problem hiding this comment.
Since this would become a part of the exported API, I think it would be best if we add comments on why you might want to (or moreover, why you probably shouldn't) use it. It's also not thread-safe if someone does decide to fiddle with things with a running embedded server.
There was a problem hiding this comment.
You know, I don't like it either, and I had started with a private variable in the server that was set it sets, but I would have had the same problem with the tests in the test package.
What I am going to do, is likely revert the changes unrelated to the JS command line parameters and on a different branch, run all tests with a debug statement in place where we ignore and have a sense of how many tests need review and go from there.
I am going to switch this PR to Draft until I have a chance to update it later today.
|
@derekcollison See my comments to Neil. I will update this PR later today and will work on the config digest "issue" separately. |
When the --js (and --store_dir) command line argument(s) are given and a configuration reload is issued, JetStream would be disabled. This is because we were not tracking the JetStream command line argument boolean, and also not merging the store_dir option. Resolves #6606 Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
|
@derekcollison @wallyqs @neilalexander Ok, I have removed the changes for the config digest and this PR is ready for review. As I mentioned, I will evaluate separately tests that would need review due to the introduction of config digest. |
|
@derekcollison Have we just disabled Travis? It did not run for this update and yet the "All checks have passed" is green... |
|
(travis should still be enabled) |
|
It should kick Travis still.. |
Ok, some issues then between GitHub and Travis. I pushed a separate branch and it did not execute yet. Will likely be resolved sooner or later. |
Includes the following (already cherry-picked) PRs: - #6587 - #6607 - #6612 - #6609 - #6620 - #6668 - #6674 - #6647 - #6684 - #6691 - #6697 - #6705 - #6706 - #6704 - #6714 - #6720 - #6727 - #6730 - #6726 - #6732 - #6759 - #6753 - #6685 - #6769 - #6777 - #6785 - #6786 - #6778 - #6790 - #6791 - #6798 - #6794 - #6801 Signed-off-by: Neil Twigg <neil@nats.io> Signed-off-by: Neil Twigg <neil@nats.io>
4 participants
When the
--js(and--store_dir) command line argument(s) aregiven and a configuration reload is issued, JetStream would be
disabled.
This is because we were not tracking the JetStream command line
argument boolean, and also not merging the store_dir option.
Resolves #6606
Signed-off-by: Ivan Kozlovic ivan@synadia.com