Support encoding.BinaryMarshaler types #75
Conversation
Sending time.Time values over libchan has resulted in panics. This is a core type and correct handling of the time type in libchan is important. By adding support for types that implement encoding.BinaryMarshaler, we get support for time.Time along with other types that implement this interface. At first, it was unclear why this correctly worked, but closer examination showed that this is supported within the codec package. The main bug was in the handling in the (*pipeReceiver).copyValue method and its spdy equivalent. The value copier really needs to be generalized and ported to work with any libchan implementation to avoid such bugs in the future.
|
@dmcgowan It might be better to use TextMarshaler for the time type or define a msgpack extension type for this functionality. Note that this is blocking ipc support in v2 registry: docker-archive/docker-registry#831. |
|
I'm in for an extension type for times.
|
|
+1 on defining an extended type for time. @stevvooe is this currently blocking any implementation or is using string a good enough work around for now? If it is we should discuss what the extended type definition is and get a PR to the spec and implementation. |
|
@dmcgowan If we change this diff to use TextMarshaler instead, I think it will meet the needs for now. Would you like me to file a ticket for "native" time support? |
|
What was the conclusion here? Did we want to just use distribution/distribution#28 is pending a resolution here. |
|
The code updated in this PR will be deprecated by #79. Do you have a timeframe for needing this feature? I will close it when the other is ready and merged. If you need it sooner we can discuss merging, I just want to ensure this doesn't require doing something special by the caller. |
|
I was just cleaning house in docker/distribution. I can link distribution/distribution#28 #79, instead. |
Sending time.Time values over libchan has resulted in panics. This is a core
type and correct handling of the time type in libchan is important. By adding
support for types that implement encoding.BinaryMarshaler, we get support for
time.Time along with other types that implement this interface.
At first, it was unclear why this correctly worked, but closer examination
showed that this is supported within the codec package. The main bug was in the
handling in the (*pipeReceiver).copyValue method and its spdy equivalent. The
value copier really needs to be generalized and ported to work with any libchan
implementation to avoid such bugs in the future.