Skip to content

Remove unused go-generate files #266

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

Open
mr-karan opened this issue Sep 13, 2022 · 5 comments
Open

Remove unused go-generate files #266

mr-karan opened this issue Sep 13, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@mr-karan
Copy link

mr-karan commented Sep 13, 2022

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 mr-karan changed the title Go Generate not updated since long time App doesn't compile after running go generate Sep 13, 2022
@mjuraga
Copy link
Collaborator

mjuraga commented Sep 13, 2022

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.

@mr-karan
Copy link
Author

mr-karan commented Sep 13, 2022

@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!

@mjuraga
Copy link
Collaborator

mjuraga commented Sep 13, 2022

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.

@mr-karan
Copy link
Author

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!

@mjuraga
Copy link
Collaborator

mjuraga commented Sep 13, 2022

That would be awsome @mr-karan. Thanks!

mr-karan added a commit to mr-karan/dataplaneapi that referenced this issue Sep 21, 2022
mr-karan added a commit to mr-karan/dataplaneapi that referenced this issue Sep 21, 2022
@mjuraga mjuraga changed the title App doesn't compile after running go generate Remove unused go-generate files Dec 1, 2022
@mjuraga mjuraga added the bug Something isn't working label Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants