Enable DrioDataSource to fetch data from warm path#61
Conversation
heidi-holm-4ss
requested review from
bjorn-einar-bjartnes-4ss and
branislav-jenco-4ss
| lables : dict | ||
| Labels and timeseries IDs as key/value pairs. | ||
| storage : str, optional | ||
| If 'warm' (default), drio is fetched from warm storage. If 'archive', data is fetched from the archive. |
There was a problem hiding this comment.
If we change default, it should be a new major version.
There was a problem hiding this comment.
Or, perhaps a new 0.8 version since we do not have 1.
There was a problem hiding this comment.
Or change it back to archive as default to avoid any issues.
There was a problem hiding this comment.
I agree with not changing defaults.
|
If we change back to 'archive' as default and perhaps mention in doc strings that 'warm' path is currently experimental in this version, and specificy that the deault of tick might be slow. |
|
@heidi-holm-4ss on a brief look it looks like the caching should still work with warm path, but have you tested it? Since the fingerprint is based on kwargs to the init of the class, then the aggregation_period and function should be part of it, as far as I can tell, so it looks ok. |
I have not tested it extensively, but I have tested that I get a new "cache file" if I change the aggregation period or method |
3 participants
This PR updates _get to support get_samples_aggregate for warm storage while keeping backward compatibility for archive.
I’m unsure whether the default storage should remain archive for backward compatibility, or if it should be set to warm by default, enabling aggregated queries out of the box. I started with warm path, and default aggregation_period tick, but when querying a full day this is much slower than going to the archive for high frequency timeseries
I'm also not sure if we should expand the documentation a bit on the DrioDataSource, it is not very specific the drio part compared to "any" data source at the moment
DrioDataSource is today at least used by odst