Skip to content
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 the ability to parse zebrad.toml config file #485

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

LarryRuane
Copy link
Collaborator

Fixes #462. If the file extension is .conf (as in zcash.conf), then parse the file as an INI file (as is done today). If the extension is .toml (as in zebrad.toml), then interpret as TOML. The only possible problem is if the user has specified a toml config file (using --zcash-conf-path) that doesn't have the .toml extension. It will then be interpreted as a .conf (INI) file. But it's not unreasonable to require the toml extension.

@LarryRuane LarryRuane added enhancement New feature or request zebra labels May 10, 2024
@LarryRuane LarryRuane requested review from nuttycom and str4d May 10, 2024 18:43
@LarryRuane LarryRuane self-assigned this May 10, 2024
@LarryRuane
Copy link
Collaborator Author

I also needed to remove a test, since it sends in a string as the config file contents (rather than a filename), and the toml parser doesn't support that. But this test did very little, and we won't miss it. If reviewers prefer, I could restore the test and have it create a file, actually it could create both a zcashd.conf file (INI format) and a zebrad.toml file and test both.

}
}

func connFromIni(confPath string) (*rpcclient.ConnConfig, error) {
cfg, err := ini.Load(confPath)
if err != nil {
return nil, fmt.Errorf("failed to read config file: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return nil, fmt.Errorf("failed to read config file: %w", err)
return nil, fmt.Errorf("failed to read config file in .conf format: %w", err)

}
_, err := toml.DecodeFile(confPath, &tomlConf)
if err != nil {
return nil, fmt.Errorf("failed to read config file: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return nil, fmt.Errorf("failed to read config file: %w", err)
return nil, fmt.Errorf("failed to read config file in .toml format: %w", err)

Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK with minor suggestion for the error messages.

@LarryRuane LarryRuane force-pushed the 2024-05-zebrad-toml branch 2 times, most recently from 2fb4e33 to 6800d6e Compare June 27, 2024 17:33
Fixes zcash#462. If the file extension is .conf (as in zcash.conf), then
parse the file as an INI file. If the extension is .toml (as in
zebrad.toml), then interpret as TOML.
@LarryRuane
Copy link
Collaborator Author

Force pushed to implement review comments (thanks, Daira), and force pushed to rebase.

@LarryRuane LarryRuane merged commit 0bd5519 into zcash:master Jun 27, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request zebra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lightwalletd does not parse zebrad.toml
2 participants