[FIXED] Processes config file during embeded server initialization.#7333
[FIXED] Processes config file during embeded server initialization.#7333yixianOu wants to merge 3 commits intonats-io:mainfrom
Conversation
Prevent configuration files from being processed twice by checking opts.configDigest before calling ProcessConfigFile in NewServer. This fixes account isolation test failures caused by duplicate configuration processing. Signed-off-by: orician <lzjzxOYX201905@gmail.com>
|
This feature was discussed in #7093, but I still believe that if the embedded NATS server could provide the capability to read configuration files, it would make embedding NATS in Go much more user-friendly. |
neilalexander
left a comment
neilalexander
left a comment
There was a problem hiding this comment.
Principally looks good to me, but I think we need a unit test to prove this behaviour. You should be able to plagiarise a good bit of TestLargeMaxPayload in opts_test.go for example but replace the LoadConfig call in the test with a config struct.
|
I am still pro |
|
Thank you for your suggestion. I will refer to TestLargeMaxPayload to write test cases for this change, and I will also explore whether NewServerFromConfig() can provide a better implementation. |
This is a good approach considering forward compatibility, but I'd like to first test whether this change might be disruptive. |
Enable embedded NATS servers to automatically process configuration files when using Options struct with ConfigFile set, addressing the limitation noted in PR nats-io#7333 comments. Changes: - Add config file processing in NewServer() when configDigest is empty - Process config files automatically for embedded servers without requiring LoadConfig - Add proper error handling for config file processing failures Tests added: - TestConfigStructReplacesLoadConfig: Validates Options struct replaces LoadConfig calls - TestConfigFileProcessingComparison: Compares LoadConfig vs Options.ProcessConfigFile behavior - TestConfigFileHotReloadWithEmbeddedServer: Ensures hot reload works with embedded servers The implementation ensures backward compatibility and doesn't interfere with existing configuration reload mechanisms. Both traditional LoadConfig and new Options struct approaches support identical functionality including hot reload capabilities. Resolves embedded server config processing as requested in PR review feedback. Signed-off-by: orician <lzjzxOYX201905@gmail.com>
Add proper error handling for config file processing failures in
|
|
Should I use |
…cessing (#7364) # Add NewServerFromConfig function for embedded server config processing ## Overview Resolves #7092 This PR introduces a new `NewServerFromConfig()` function that automatically processes configuration files for embedded NATS servers. ## Problem When using NATS embedded in Go applications, developers (such as me) often pass the `ConfigFile` field in the `Options` struct, but `NewServer(opts *Options)` doesn't automatically process this configuration file. This leads to confusion and requires additional manual steps to load the configuration. ## Solution ### Approach Comparison In pr #7333, a solution was proposed to directly modify `NewServer()`: ```go func NewServer(opts *Options) (*Server, error) { if opts.ConfigFile != _EMPTY_ && opts.configDigest == "" { if err := opts.ProcessConfigFile(opts.ConfigFile); err != nil { return nil, err } } // ...original code } ``` However, directly modifying `NewServer()` might raise concerns about backward compatibility. Therefore, this PR introduces a dedicated `NewServerFromConfig()` function specifically to handle the `ConfigFile` field of `*Options`. ### Implementation This PR adds: ```go func NewServerFromConfig(opts *Options) (*Server, error) { if opts.ConfigFile != _EMPTY_ && opts.configDigest == "" { if err := opts.ProcessConfigFile(opts.ConfigFile); err != nil { return nil, err } } return NewServer(opts) } ``` ## Testing Added comprehensive test coverage: - **`TestNewServerFromConfigFunctionality`**: Tests basic functionality and error handling scenarios - **`TestNewServerFromConfigVsLoadConfig`**: Validates equivalence with traditional `LoadConfig()` approach using deep equality checks ## Acknowledgments Thanks to @ripienaar for the suggestion to create a separate function instead of modifying `NewServer()` directly. ## Signing Off Signed-off-by: orician lzjzxOYX201905@gmail.com
3 participants
Resolves #7092
Edited the NewServer function in the server package , server.go file. Using the ProcessConfigFile method of the Options type, to enable the embeded nats-server to read the config file
Prevent configuration files from being processed twice by checking opts.configDigest before calling ProcessConfigFile in NewServer.
Signed-off-by: orician lzjzxOYX201905@gmail.com