[CLD-781]: fix(datstore): change datastore type to enum #534
Conversation
🦋 Changeset detectedLatest commit: 6405ee0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Instead of using string , we use enum to represent the location of the datastore data. Also exposes the location in the field of the Config struct so it can be used to selec the right client to load.
graham-chainlink
force-pushed
the
ggoh/cld-781/datastore-enum
branch
from
d2c0f8c to
6405ee0
Compare
graham-chainlink
changed the title
| Jira *cfgjira.Config | ||
|
|
||
| // DatastoreType specifies the type of datastore to use (either "file" or "catalog"). | ||
| DatastoreType cfgdomain.DatastoreType |
There was a problem hiding this comment.
This will be used in Load Environment to load the right data from the selected datastore
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the datastore configuration from a string type to a strongly-typed enum (DatastoreType), improving type safety and code clarity. The change introduces DatastoreTypeFile and DatastoreTypeCatalog constants and exposes the datastore type in the Config struct for downstream use.
Key Changes:
- Introduced
DatastoreTypeenum with validation methods to replace string-based datastore representation - Added
LoadDatastoreTypefunction to retrieve datastore configuration from domain files - Exposed
DatastoreTypefield in the mainConfigstruct for client selection
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| engine/cld/config/domain/domain.go | Defines DatastoreType enum with constants, validation methods, and updates Environment struct to use the new type |
| engine/cld/config/domain/domain_test.go | Updates tests to use DatastoreType enum constants instead of string literals |
| engine/cld/config/env.go | Adds LoadDatastoreType function to load and extract datastore configuration |
| engine/cld/config/env_test.go | Adds comprehensive test coverage for LoadDatastoreType function |
| engine/cld/config/config.go | Updates Config struct to include DatastoreType field and loads it during initialization |
| engine/cld/config/config_test.go | Adds assertion to verify DatastoreType is loaded in configuration |
| .changeset/good-chairs-allow.md | Documents the change in the changeset |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
| for name, env := range cfg.Environments { | ||
| if env.Datastore == "" { | ||
| env.Datastore = "file" // Default to file if not specified | ||
| env.Datastore = DatastoreTypeFile // Default to file if not specified |
There was a problem hiding this comment.
I'm not 100% convinced on implicitly defaulting to DatastoreTypeFile inside the code as it seems too much hidden from the user. I'd prefer to always have an explicit setting in domain.yaml. Keeping it in the code for now is ok because we can release without problems but I'd add a TODO to remove that and always require a config once we add a domain.yaml config for all domain environments. Thoughts?
There was a problem hiding this comment.
yeah i was thinking to maintain backwards compatibility for now, once once everyone is moved to catalog, we can enforce the config i guess. I added a todo for now
2 participants
Instead of using string , we use enum to represent the location of the datastore data.
Also exposes the location in the field of the Config struct so it can be used to select the right client to load.
JIRA: https://smartcontract-it.atlassian.net/browse/CLD-781