Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

timeShift function: adequately set QueryFrom/QueryTo on output series #1946

Closed
wants to merge 3 commits into from

Conversation

fkaleo
Copy link
Contributor

@fkaleo fkaleo commented Nov 11, 2020

timeShift function: adequately set QueryFrom/QueryTo on output series (a5144cd) which were previously passed as is.
The unit tests were updated to check on that (b225ccb) which also required moving and generalising the check for input not being changed by the function execution (0bdd9e0)

Fix #1939

@fkaleo fkaleo requested a review from Dieterbe November 11, 2020 20:02
@stale
Copy link

stale bot commented Feb 13, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 13, 2021
@stale stale bot closed this Mar 19, 2021
@fkaleo fkaleo reopened this Mar 20, 2021
@stale stale bot removed the stale label Mar 20, 2021
@Dieterbe
Copy link
Contributor

@fkaleo did you fully look into how QueryFrom and QueryTo are used everywhere throughout metrictank?
IIRC they're tricky because the values get copied and have long lasting effects across the entire lifetime of the response and possibly beyond (e.g. if we cache the values). It's not fresh on my mind right now but that was always my main concern with this.

@fkaleo
Copy link
Contributor Author

fkaleo commented Mar 22, 2021

@fkaleo did you fully look into how QueryFrom and QueryTo are used everywhere throughout metrictank?
IIRC they're tricky because the values get copied and have long lasting effects across the entire lifetime of the response and possibly beyond (e.g. if we cache the values). It's not fresh on my mind right now but that was always my main concern with this.

Frankly I don't remember but I think my former self did.

@stale
Copy link

stale bot commented Jun 20, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 20, 2021
@stale stale bot closed this Jun 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic in function processing (seriesaggregators.go)
2 participants