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

Bugs when properties are specified as metadata of struct #32

Open
rohans81 opened this issue May 18, 2018 · 5 comments
Open

Bugs when properties are specified as metadata of struct #32

rohans81 opened this issue May 18, 2018 · 5 comments

Comments

@rohans81
Copy link

rohans81 commented May 18, 2018

There are some bugs when we specify properties as part of the struct metadata.
e.g

config file

graphite.host=localhost
graphite.metric.name=gtest

cloud.tenant.name=xxx-qa
cloud.keystone.url=https://xxx.zzz.ccc.aaa.xx/v2.0
cloud.swift.username=yyy-bb-ccc
cloud.swift.passwd=changeme

kafka.topic=ttt
kafka.servers=localhost:9092,zzz.dev.yyy.ca:9092
kafka.consumergroup=test
kafka.consumeFromStart=true

bulk.delete.max_wait=10s
log.level=debug
// Configuration holds external configuration for the consumers
type Configuration struct {
	CloudTenant           string              `properties:"cloud.tenant.name"`
	KeystoneURL           string              `properties:"cloud.keystone.url"`
	Username              string              `properties:"cloud.swift.username"`
	Userpwd               string              `properties:"cloud.swift.passwd"`
	Servers               []string			  `properties:"kafka.servers"`
	Cgroupname            string              `properties:"kafka.consumergroup"`
	Topic                 string              `properties:"kafka.topic"`
	ConsumeFromStart      bool                `properties:"kafka.consumeFromStart,default=false"`
        KafkaVersion          sarama.KafkaVersion `properties:"kafka.version,default=0.11.0.2"`
	Port                  int                 `properties:"port,default=8383"`
	Graphite              string              `properties:"graphite.host, default=carbon.service.consul"`
	GraphiteMetricName    string              `properties:"graphite.metric.name"`
	LogLevel              string              `properties:"log.level,default=info"`
	BulkDeleteMaxItems    int                 `properties:"bulk.delete.max_items,default=100"`
	BulkDeleteMaxWaitTime time.Duration       `properties:"bulk.delete.max_wait,default=5s"`
	

}

It fails on the first field/property it has a issue within the struct. All fields after this filed (in struct order) are not processed. e.g if the field KafkaVersion could not be decoded then all fields below it will also not be decoded.

There are issues in following cases
1> The property is missing in property file and no default value is mentioned in the struct
2> if the field type is not supported by decoder eg (sarama.KafkaVersion)
3> if the fields in the struct do not have the property tag and a property with the field name is not present in the property file.

Is there any way we can exclude a field from decode logic?
Regards
Rohan

@magiconair
Copy link
Owner

magiconair commented Jun 13, 2018

Fields with properties:"-" are ignored but that would still be a bug.

@magiconair
Copy link
Owner

When I try this specific example Decode returns an error. Do you check all the errors?

Added this to decode_test.go

func TestDecodeSkipFail(t *testing.T) {
	type S struct {
		V sarama.KafkaVersion `properties:"kafka.version,default=0.11.0.2"`
		S string
	}
	in := `S=some value`
	out := &S{sarama.V0_11_0_2, "some value"}
	testDecode(t, in, &S{}, out)
}
--- FAIL: TestDecodeSkipFail (0.00s)
	decode_test.go:298: got cannot set version want nil

@rohans81
Copy link
Author

yes. I had missed checking the error.
It would be helpful to add the details around ignoring properties to README file.
Thanks.

@magiconair
Copy link
Owner

magiconair commented Jun 16, 2018

This should be in the godoc or isn’t it?

@magiconair
Copy link
Owner

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

2 participants