specgen: re-enable unit tests #217
Conversation
hariso
force-pushed
the
haris/specgen-fix-other-tests
branch
from
ac63a46 to
2c6046d
Compare
hariso
force-pushed
the
haris/specgen-fix-other-tests
branch
from
2c6046d to
ec9fe4d
Compare
| @@ -126,7 +126,7 @@ type DestinationWithBatch struct { | |||
| // Maximum size of batch before it gets written to the destination. | |||
| BatchSize int `json:"sdk.batch.size" default:"0" validate:"gt=-1"` | |||
| // Maximum delay before an incomplete batch is written to the destination. | |||
| BatchDelay time.Duration `json:"sdk.batch.delay" default:"0" validate:"gt=-1"` | |||
| BatchDelay time.Duration `json:"sdk.batch.delay" default:"0"` | |||
There was a problem hiding this comment.
Why removing the previous validation?
There was a problem hiding this comment.
Because it actually isn't something we support: #54. I guess that Lovro was initially experimenting with it, and found that we don't support gt and lt validations for time.Duration.
| @@ -139,6 +139,10 @@ type SourceWithSchemaExtraction struct { | |||
| // todo: use https://github.com/ConduitIO/conduit-commons/issues/142 to parse | |||
| // the string value into schema.Type directly. | |||
| func (c *SourceWithSchemaExtraction) SchemaType() schema.Type { | |||
| if c.SchemaTypeStr == "" { | |||
| return schema.TypeAvro | |||
There was a problem hiding this comment.
(thinking out loud) Could we initialize it with this value when instantiating SourceWithSchemaExtraction?
There was a problem hiding this comment.
I think that would work if we constraint ourselves to always use a constructor method. It's not terrible, but unless we make the type unexported we can't really force anyone to use the constructor method. I believe that in most cases this should be enough, because when the config is parsed using the sdk.Util.ParseConfig function, the default should be set (because we have the default tag).
Description
Closes #214.
Quick checks: