Some of the timeout style properties should be TimeSpan

[jterry75]

For example:

Config.StopTimeout
HealthConfig.Interval
HealthConfig.Timeout
etc.

This will require us to implement custom JsonConverters for TimeSpan but because sometimes the result is in Seconds and in others its NanoSeconds we likely need to introduce an attribute that describes the destination conversion expected.

[jasoncouture]

Just a comment on this rather than opening a new issue:
the underlying HttpClient timeout should be exposed, there are numerous situations where it would be highly helpful to be able to change the default timeout of 100 seconds.

client.RequestTimeout or configuration.RequestTimeout are a few suggestions.

[jterry75]

@jasoncouture - Actually these arent the same type of "timeout" issue. Can you open a new issue for this so we can track it independently?

[jasoncouture]

@jterry75 Sorry for the late response, Sure thing.

[orthros]

I just started looking at this this morning.
In addition to specgen getting updated with the custom type mapping, I'm assuming we should have the custom Json Serialization attribute mapped there as well?

[orthros]

So I've got a rough draft of this working, though it is untested.

specgen is now outputting certain types to TimeSpan? and you can specify on a per-property basis custom Attributes to decorate the property with.

On the json serialization of TimeSpan attributes: would it be better to decorate the Property with the "standard" [JsonConvert(Type converterType)] style, or to put in a IContractResolver and custom Attributes on the Properties to denote what should be serialized to/from seconds/nanoseconds?

I have it working both ways, but prefer the custom attributes path to help keep the consuming code clean.

[jasoncouture]

@orthros,
Just my $0.02, I think the attribute is the better choice, because if we ever needed to change the contract resolver for something else it would be a headache.

[orthros]

@jasoncouture thanks for the feedback! In my PR (#205 ), I wound up going with a single attribute that you can decorate with a Seconds or Nanoseconds enumeration like so

[DataMember(Name = "Timeout", EmitDefaultValue = false)]
[TimeSpanSerialization(SerializationTarget.Nanoseconds)]
public TimeSpan Timeout { get; set; }

which then gets intercepted in the ContractResolver.

I wanted to avoid coupling the contract too closely with the serializer, and in his initial description @jterry75 mentioned introducing an Attribute, which I interpreted to be "custom" attribute 😄

I do see your point though; messing with the ContractResolver can be opaque at times.

'twould be a quick fix to refactor the code to use the [JsonConvert(SecondsConverter)] instead of the custom attributes if ya'll would rather do it that way.

[orthros]

I just filed another pull request that uses the builtin [JsonConvert] attributes and adds a small enhancement to specgen.

[jterry75]

@orthros - This is completed yes? Can we close?

[orthros]

Yes it is. Thank you!