Skip to content

Revert "query/api: properly pass downsampling param (#1144)"#1154

Merged
bwplotka merged 1 commit intomasterfrom
revert-1144
May 17, 2019
Merged

Revert "query/api: properly pass downsampling param (#1144)"#1154
bwplotka merged 1 commit intomasterfrom
revert-1144

Conversation

@bwplotka
Copy link
Copy Markdown
Member

@bwplotka bwplotka commented May 16, 2019

If I am not wrong downsampling on master is broken for 2days.

This reverts commit 7a767ef.

See: #1144 (comment)

Reason:

Tests are nice but they are assuming no-one converts resolution in further stack. Let's revert this and fix the getFor as it's done here: #1146

This reverts commit 7a767ef.

# Conflicts:
#	CHANGELOG.md
@brancz
Copy link
Copy Markdown
Member

brancz commented May 17, 2019

Just curious, wouldn't it make sense to rather remove the conversions lower in the stack?

@GiedriusS
Copy link
Copy Markdown
Member

That's what I was talking about in the other PR. Because otherwise IMHO we would just be kicking the can further down the road until someone in the future again will get confused about what actual unit is being used. So I propose just using int64 and everywhere else consistently.

@bwplotka
Copy link
Copy Markdown
Member Author

Still, can we revert first and then talk about improvements? (:

Copy link
Copy Markdown
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

Okay, so let's revert it and fix that interface in the new PR. Sorry for the inconvenience, I was completely oblivious to the querier code which divides this value once more

@bwplotka bwplotka merged commit 02ce8af into master May 17, 2019
@bwplotka bwplotka deleted the revert-1144 branch May 17, 2019 08:52
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

Successfully merging this pull request may close these issues.

3 participants