cdm timeseries dog-4175 #261
Conversation
| /// Id of the instance. | ||
| pub id: InstanceId, | ||
| /// An audit of the lifecycle of the instance | ||
| pub audit: CogniteAuditable, |
There was a problem hiding this comment.
You don't appear to use this field.
There was a problem hiding this comment.
This field can't be set by the user technically, because it is meant to give the user information about the lifecycle of the instance.
There was a problem hiding this comment.
Ah, is it something generated by FDM?
There was a problem hiding this comment.
Yes they are i.e. created_time, last_updated_time and deleted_time.
| auto_create_start_nodes: Option<bool>, | ||
| auto_create_end_nodes: Option<bool>, | ||
| skip_on_version_conflict: Option<bool>, | ||
| replace: Option<bool>, |
There was a problem hiding this comment.
Generally, I find that Option<bool> should be avoided. In this case it's better if users just have to pick one, either true or false, leaving it as None as no special semantic meaning beyond "use the default", so if users want to leave them as None they have to look up what that means, which means they might as well just set a value.
We should have a discussion on this. |
cognite/tests/data_models_tests.rs
Outdated
| let timeseries_del = timeseries_del.items.first().unwrap(); | ||
| assert!(matches!(timeseries_del, NodeOrEdgeSpecification::Node(_))); | ||
|
|
||
| // No need to ensure unit delete because it gets deleted with timeseries apparently. |
There was a problem hiding this comment.
I don't think it gets deleted. But I think you need something special to be able to retrieve it. Specifying the space in the query or something.
Also, I don't think it should be possible to create custom units. A unit should at least have some information about which quantity it is supposed to measure: length, weight, etc. And a conversion factor.
If all you want to test is that you can point to a unit from a time series, could you instead use the cdf_cdm_units/temperature:deg_c unit?
There was a problem hiding this comment.
@sighol I've tried fetching the unit suggested but doesn't seem to exist. Is this a view or container?
There was a problem hiding this comment.
You don't need to fetch it, you can just set it.
No description provided.