Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove Aggregate.dict(), TimeValue.dict(). #142

Open
bmario opened this issue Feb 19, 2023 · 3 comments
Open

remove Aggregate.dict(), TimeValue.dict(). #142

bmario opened this issue Feb 19, 2023 · 3 comments

Comments

@bmario
Copy link
Member

bmario commented Feb 19, 2023

This method is used in parsing HistoryResponse in the metricq-grafana endpoint for the timeline request.
The active_time attribute would allow us to draw the last (or only) aggregate in the MetricQ-Webview.

@tilsche
Copy link
Contributor

tilsche commented Mar 2, 2023

I'm not sure I even like the dict there at all. I mean sure we can add active_time. But in general it seems arbitrary whether to use mean, and/or sum, integral etc... Isn't that more consumer dependent? What's the advantage to have that decision encoded in the interface?

@bmario
Copy link
Member Author

bmario commented Mar 2, 2023

Actually, we don't touch the dict in metricq-grafana, but pass that down as is to metricq-js. That was the whole point of the timeline endpoint/query there

@tilsche
Copy link
Contributor

tilsche commented Mar 6, 2023

We discussed to deprecate / delete the dict function and do the conversion in metricq-grafana

@tilsche tilsche changed the title Add active_time to Aggregate.dict() remove Aggregate.dict(), TimeValue.dict(). Mar 6, 2023
tilsche added a commit that referenced this issue Apr 13, 2023
tilsche added a commit that referenced this issue Apr 13, 2023
tilsche added a commit that referenced this issue Apr 14, 2023
tilsche added a commit that referenced this issue Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants