Skip to content
This repository was archived by the owner on Apr 2, 2024. It is now read-only.

Conversation

JamesGuthrie
Copy link
Member

@JamesGuthrie JamesGuthrie commented Apr 11, 2022

A couple of typos in this function broke it when used with two-step
continuous aggregates. A companion change in the promscale repository
adds a test to exercise this code path 1.

Copy link
Contributor

@sumerman sumerman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch! LGTM overall. Although, it'd be great if you added a regression test.

Copy link
Contributor

@cevian cevian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM not sure why this wasn't caught by the connector's e2e tests. Maybe ping @antekresic who worked on this feature?

@JamesGuthrie
Copy link
Member Author

LGTM not sure why this wasn't caught by the connector's e2e tests. Maybe ping @antekresic who worked on this feature?

It's not covered at all in the connector e2e tests. I'm going to add a test for it before I merge this change.

@JamesGuthrie
Copy link
Member Author

Ouch! LGTM overall. Although, it'd be great if you added a regression test.

Yes, thanks to the regression test I found another bug/typo.

JamesGuthrie added a commit to timescale/promscale that referenced this pull request Apr 12, 2022
In [1] we fixed some typos in the `reset_metric_retention_period`
function which caused the function to break when used with two-step
continuous aggregates.

This change adds tests cases to exercise `reset_metric_retention_period`
with two-step aggregates.


[1]: timescale/promscale_extension#180
A couple of typos in this function broke it when used with two-step
continuous aggregates. A companion change in the promscale repository
adds a test to exercise this code path [1].

[1]: timescale/promscale#1294
@JamesGuthrie JamesGuthrie force-pushed the jg/fix-typo-idempotent-base branch from 3b9da52 to febca77 Compare April 12, 2022 12:44
@JamesGuthrie JamesGuthrie changed the title Fix typo in idempotent script 001-base.sql Fix prom_api.reset_metric_retention_period Apr 12, 2022
@JamesGuthrie JamesGuthrie merged commit ed27edf into develop Apr 13, 2022
@JamesGuthrie JamesGuthrie deleted the jg/fix-typo-idempotent-base branch April 13, 2022 07:44
JamesGuthrie added a commit to timescale/promscale that referenced this pull request Apr 13, 2022
In [1] we fixed some typos in the `reset_metric_retention_period`
function which caused the function to break when used with two-step
continuous aggregates.

This change adds tests cases to exercise `reset_metric_retention_period`
with two-step aggregates.


[1]: timescale/promscale_extension#180
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants