dataplaneapi icon indicating copy to clipboard operation
dataplaneapi copied to clipboard

Remove unused go-generate files

Open mr-karan opened this issue 2 years ago • 5 comments

I ran go run generate/go-generate.go $(pwd) to generate updated models and configuration files. It generated a file configuration_generated.go (as it seems to be hardcoded here: https://github.com/haproxytech/dataplaneapi/blob/master/generate/go-generate.go#L356)

I manually renamed configuration_generated.go => configuration_storage.go and ran make build. I found the app doesn't compile anymore:

configuration/configuration_storage.go:65:14: undefined: log
configuration/configuration_storage.go:187:16: cfgStorage.LogTargets undefined (type *StorageDataplaneAPIConfiguration has no field or method LogTargets)
configuration/configuration_storage.go:188:18: cfg.LogTargets.Store undefined (type "github.com/haproxytech/dataplaneapi/log".Targets has no field or method Store)
configuration/configuration_storage.go:188:36: cfgStorage.LogTargets undefined (type *StorageDataplaneAPIConfiguration has no field or method LogTargets)
configuration/configuration_storage.go:362:13: cfgStorage.LogTargets undefined (type *StorageDataplaneAPIConfiguration has no field or method LogTargets)
configuration/configuration.go:292:7: cfg.LogTargets undefined (type *StorageDataplaneAPIConfiguration has no field or method LogTargets)
configuration/file-storage-hcl.go:79:37: invalid argument: localCopy.Cluster.ClusterLogTargets (variable of type *[]*models.ClusterLogTarget) for len
configuration/file-storage-hcl.go:81:29: cannot range over localCopy.Cluster.ClusterLogTargets (variable of type *[]*models.ClusterLogTarget)
configuration/file-storage-hcl.go:82:39: invalid operation: cannot index localCopy.Cluster.ClusterLogTargets (variable of type *[]*models.ClusterLogTarget)
configuration/file-storage-hcl.go:85:15: localCopy.LogTargets undefined (type StorageDataplaneAPIConfiguration has no field or method LogTargets)
configuration/file-storage-hcl.go:85:15: too many errors
make: *** [Makefile:24: build] Error 2

The old file had this struct:

type configTypeLog struct {
	LogTo     *string           `yaml:"log_to,omitempty" hcl:"log_to,omitempty"`
	LogFile   *string           `yaml:"log_file,omitempty" hcl:"log_file,omitempty"`
	LogLevel  *string           `yaml:"log_level,omitempty" hcl:"log_level,omitempty"`
	LogFormat *string           `yaml:"log_format,omitempty" hcl:"log_format,omitempty"`
	ACLFormat *string           `yaml:"apache_common_log_format,omitempty" hcl:"apache_common_log_format,omitempty"`
	Syslog    *configTypeSyslog `yaml:"syslog,omitempty" hcl:"syslog,omitempty"`
}

The new generated file has this struct but the fields are different:

type configTypeLog struct {
	LogTargets *log.Targets `yaml:"log_targets,omitempty" hcl:"log_targets,omitempty"`
}

It's also missing the LogTargets from StorageDataplaneAPIConfiguration struct.

Please let me know if this is a bug or I am doing something wrong!

Thanks

mr-karan avatar Sep 13 '22 07:09 mr-karan

Hi, to generate updated models and operations, I suggest you use our Makefile target generate. Or if you are doing something custom use this for inspiration: https://github.com/haproxytech/dataplaneapi/blob/master/Makefile#L31

We don't use generate/go-generate to generate config stuff no more. That is why we renamed configuration_generated.go => configuration_storage.go. You have to manually add changes now to it.

We should remove the go-generate.go file there to remove the confusion.

mjuraga avatar Sep 13 '22 08:09 mjuraga

@mjuraga Hi! Actually I did use make generate-native (as noted in the contrib guide). The reason is that I've some changes in specifications in client-native so I generated new models with make models in that repository and did a go mod replace in dataplaneapi.

When I ran make generate-native, the configuration_storage.go didn't contain the new models. (I am trying to add support for Nomad service discovery):

type configTypeServiceDiscovery struct {
	Consuls    *[]*models.Consul    `yaml:"consuls,omitempty" hcl:"consuls,omitempty"`
	AWSRegions *[]*models.AwsRegion `yaml:"aws_regions,omitempty" hcl:"aws_regions,omitempty"`
}

However, when I ran generate/go-generate I found this to be present:

type configTypeServiceDiscovery struct {
	Consuls    *[]*models.Consul    `yaml:"consuls,omitempty" hcl:"consuls,omitempty"`
	Nomads     *[]*models.Nomad     `yaml:"nomads,omitempty" hcl:"nomads,omitempty"`
	AWSRegions *[]*models.AwsRegion `yaml:"aws_regions,omitempty" hcl:"aws_regions,omitempty"`
}

So, do you suggest I edit the configuration_storage.go manually for now? That's totally fine by me, I just was concerned that this file shouldn't get overwritten in future if go generate is run again.

Thanks!

mr-karan avatar Sep 13 '22 08:09 mr-karan

Yes, you should change the file manually, we won't be running go generate again. It had some issues with these kind of fields, and we decided it was easier to just add stuff manually then to rewrite the go-generate.go file.

mjuraga avatar Sep 13 '22 08:09 mjuraga

Thanks a lot for the quick response, appreciate it. I'll close this :)

P.S. It might be a good idea to remove the // Code generated by go generate; DO NOT EDIT. so editors like VScode don't complain :)

image

I can send a PR for removing go-generate.go and these lines if you want!

mr-karan avatar Sep 13 '22 08:09 mr-karan

That would be awsome @mr-karan. Thanks!

mjuraga avatar Sep 13 '22 08:09 mjuraga