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

Consider minimizing the UpdateConfiguraton API. #45

Open
rompetroll opened this issue Jul 23, 2024 · 0 comments
Open

Consider minimizing the UpdateConfiguraton API. #45

rompetroll opened this issue Jul 23, 2024 · 0 comments

Comments

@rompetroll
Copy link
Contributor

Layers are currently required to implement UpdateConfiguration(config *common.Config) common.LayerError, and in addition Dataset(dataset string) (common.Dataset, common.LayerError).

The reason for this is that the specific implementation details of a layer dataset cannot be known to the common framework.

Given that a typical implementation of UpdateConfiguration looks like this:

func (dl *SomeDatalayer) UpdateConfiguration(config *common.Config) common.LayerError {
	existingDatasets := map[string]bool{}
	// update existing datasets
	for k, v := range dl.datasets {
		for _, dsd := range config.DatasetDefinitions {
			if k == dsd.DatasetName {
				existingDatasets[k] = true
				v.datasetDefinition = dsd
			}
		}
	}
	// remove deleted datasets
	for k := range dl.datasets {
		if _, found := existingDatasets[k]; !found {
			delete(dl.datasets, k)
		}
	}

	// add new datasets
	for _, dsd := range config.DatasetDefinitions {
		if _, found := existingDatasets[dsd.DatasetName]; !found {
			dl.datasets[dsd.DatasetName] = &Dataset{
				logger:            dl.logger,
				db:                dl.db,
				datasetDefinition: dsd,
			}
		}
	}

	return nil
}

and a typical Dataset implementation looks like this

func (dl *SomeDatalayer) Dataset(dataset string) (common.Dataset, common.LayerError) {
	ds, found := dl.datasets[dataset]
	if found {
		return ds, nil
	}
	return nil, ErrDatasetNotFound(dataset)
}

... it seems that all the layer really should need to provide is how to map a DatasetDefinition to a custom Dataset instance. And then the common framework can provide UpdateConfiguration and a Dataset impl under the hood.
The framework can then also automatically call UpdateConfiguration as part of the bootstap invocation, currently users have to remember to call it once as part of their layer constructor.

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

1 participant