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

Enhancement: Config settings with dependencies should have logical checks in place #124

Open
osteensco opened this issue Sep 11, 2024 · 1 comment

Comments

@osteensco
Copy link
Contributor

PROBLEM:
Currently, it's possible to do something like the following

        config := echovault.DefaultConfig()
        config.EvictionPolicy = constants.AllKeysLRU
	Server, err := echovault.NewEchoVault(
		echovault.WithConfig(config),
	)

Because default setting for MaxMemory is 0, this causes an error when trying to interact with the server. This may not be entirely clear to a new user.

PROPOSED SOLUTION:
We should either put in logical checks when passing in configs on instantiating a server, or create separate default values for settings that are naturally coupled like MaxMemory and EvictionPolicy.

@kelvinmwinuka
Copy link
Collaborator

kelvinmwinuka commented Sep 11, 2024

I like your proposed solution. I think we should have multiple sets of defaults and also create a validation function to solve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants