Some of the timeout style properties should be TimeSpan about docker.dotnet HOT 10 CLOSED

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.

from docker.dotnet.

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

from docker.dotnet.

@jterry75 Sorry for the late response, Sure thing.

from docker.dotnet.

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?

from docker.dotnet.

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.

from docker.dotnet.

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

from docker.dotnet.

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

from docker.dotnet.

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

from docker.dotnet.

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

from docker.dotnet.

Yes it is. Thank you!

from docker.dotnet.