EchoVault / SugarDB

Embeddable and distributed in-memory alternative to Redis.
https://sugardb.io
Apache License 2.0
404 stars 21 forks source link

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

Open osteensco opened 1 month ago

osteensco commented 1 month ago

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 commented 1 month ago

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