Fix bug with conversion of Timestamp::Microseconds to chrono::Datetime

[amonin7]

Description

Fix bug with the conversion of Timestamp::Microseconds to chrono::Datetime
Fix for issue #133

Checklist

• Formatted code using cargo fmt --all
• Linted code using clippy
  • with reqwest feature: cargo clippy --manifest-path influxdb/Cargo.toml --all-targets --no-default-features --features use-serde,derive,reqwest-client -- -D warnings
  • with surf feature: cargo clippy --manifest-path influxdb/Cargo.toml --all-targets --no-default-features --features use-serde,derive,hyper-client -- -D warnings
• Updated README.md using cargo doc2readme -p influxdb --expand-macros
• Reviewed the diff. Did you leave any print statements or unnecessary comments?
• Any unfinished work that warrants a separate issue captured in an issue with a TODO code comment

[amonin7]

FYI

• checked the code with cargo clippy and had no warnings related to the new code, however, there were warnings related to the previous code. That is why did not checked those marks.

• did not update the readme, since this is a minor bugfix, but please let me know if I should

[msrd0]

Yeah don't worry about warnings you didn't introduce. Please run rustfmt though.

I was gonna comment that we should probably use the Utc.timestamp_micros etc functions but those somehow return a result whereas timestamp_nanos does not ... chrono is such a mess, they really need to do a breaking change release and fix up their api but for now I guess we just keep calling timestamp_nanos.

[msrd0]

Oh wow we even had a broken test case for micros. I guess you'll need to fix/remove that one.

[amonin7]

I could see that the timestamp_nanos uses unwrap inside itself, so that is why it returns Datetime instead of Result

    fn timestamp_nanos(&self, nanos: i64) -> DateTime<Self> {
        ...
        self.timestamp_opt(secs, nanos as u32).unwrap()
    }

So since we know, what precision each type has (e.g. Timestamp::milliseconds for sure has ms precision), probably we can call those timestamp_microseconds, etc. with the unwrap after it in place. This is just a suggestion :)

[amonin7]

Oh wow we even had a broken test case for micros. I guess you'll need to fix/remove that one.

Yeah, I did not remove it, since did not know, if somebody needed it for some reason... I can remove it if you want

[msrd0]

Yeah, I did not remove it, since did not know, if somebody needed it for some reason... I can remove it if you want

Well the test divides 1 by some number greater than 1, so it gets integer'ed to 0. Looks to me like the test was just as broken as the impl. I'd probably just replace the existing test with your new test.

[msrd0]

Also please remove the MICRO_PER_NANO constant, that should be 0.0001 or something like that, not 1000.

[msrd0]

Thanks

[amonin7]

Also, as I mentioned, cargo clippy warns about using deprecated chronology method:
warning: use of deprecated method chrono::DateTime::<Tz>::timestamp_nanos: use timestamp_nanos_opt() instead
--> influxdb/src/query/mod.rs:94:42
idk, if this is helpful, but you can use

opt with unwrap to fix this clippy warning, like this

    fn from(date_time: DateTime<T>) -> Self {
       Timestamp::Nanoseconds(date_time.timestamp_nanos_opt().unwrap() as u128)
   }

[msrd0]

Influxdb 0.7.2 has been released with this fix.

[amonin7]

Thanks for letting me know! Will be happy to use the correct impl :)

[amonin7]

p.s. I've seen that you have also corrected the impl from as I suggested
Happy to help :)

[msrd0]

Yeah I knew that was a thing and easy to fix I'd just been too lazy to do that until now 😅