Parametrize InfluxDB Version

[Empty2k12]

Description

More and more feature keep getting added or wished for that depend on specific versions of InfluxDB (#132, #136, #165). This PR creates a PhantomData version field in the client. That way, we can implement traits for version specific features and do not mix feature that might be incompatible with a connected database.

I am looking for first feedback about this design / approch before I port over any other version related code.

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 serde,derive,reqwest-client-rustls -- -D warnings
  • with surf feature: cargo clippy --manifest-path influxdb/Cargo.toml --all-targets --no-default-features --features 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

[msrd0]

I like the idea behind this change, but I am not sure if the current approach, i.e. always having a version attached, is necessary. We should probably have an AnyVersion-like type that works with any influxdb server version we support. This, of course, would not support features that don't work with all influxdb server versions, but from experience hosting services that connect to postgres, I expect them to work with multiple recent versions without recompilation.

[msrd0]

I don't think this change makes much sense, given that we already decided to sunset surf support in #147

[Empty2k12]

Yeah, I was thinking of doing #147 first.

[msrd0]

I see, still not sure why the generic?

[Empty2k12]

I expect them to work with multiple recent versions without recompilation

I was thinking about things like parametrized queries, where we can build a V3 query serializer / client that is capable of doing that and fallback to one which does not do that on v1/v2. Is that possible with your approach? I am open to solving this another way or better, this is just the first thing I came up with.

[msrd0]

If you bake it into the type signature, you cannot change that at runtime. If you want to adjust the behaviour at runtime, I'm not sure why you'd bake it into the type signature?

Your approach might make sense, but I don't see it yet

[Empty2k12]

By using the marker, we can implement something like this. Unless it's a Client<InfluxVersion2>, you cannot use .with_retention_policy().

pub trait RetentionPolicy {
    fn with_retention_policy<S>(self, retention_policy: S) -> Self;
}

impl<H> RetentionPolicy for Client<InfluxVersion2, H> {
    pub fn with_retention_policy<S>(mut self, retention_policy: S) -> Self
    where
        S: Into<String>,
    {
        let mut with_retention_policy = self.parameters.as_ref().clone();
        with_retention_policy.insert("rp", retention_policy.into());
        self.parameters = Arc::new(with_retention_policy);
        self
    }
}

[msrd0]

Yes. But, what if I want to write an application that supports all versions of influxdb, and wants to set the retention policy if supported? In that case, I'd much rather have the method always available, and return something like Err(IncompatibleVersion) at runtime which I can then handle appropriately.

[Empty2k12]

Got it, that indeed makes sense. After #168 merged, I will refactor this.