flavray / avro-rs

Avro client library implementation in Rust
MIT License
169 stars 95 forks source link

Support ISO 8601 parsing to TimestampMicros #165

Open KannarFr opened 3 years ago

KannarFr commented 3 years ago

Why?

TimestampMicros expected, got String

Reason

fn resolve_timestamp_micros(self) -> Result<Self, Error> {
  match self {
      Value::TimestampMicros(ts) | Value::Long(ts) => Ok(Value::TimestampMicros(ts)),
      Value::Int(ts) => Ok(Value::TimestampMicros(i64::from(ts))),
      other => Err(Error::GetTimestampMicros(other.into())),
  }
}

Suggestion

Try convert String to ISO 8601 and then to long for TimestampMicros and TimestampMillis avro impl.

@flavray WDYT?

flavray commented 3 years ago

It looks like this would go against the Avro specification (if I understand your request correctly, feel free to correct me if I'm wrong!). I would be inclined to try to keep the implementation as close to the specification as possible, in order to make operability with other implementations of Avro (be it in rust or other languages) less tricky. @poros do you have any opinion?