Skip to content

Conversation

@colega
Copy link
Contributor

@colega colega commented May 6, 2022

What this PR does:

Exports the ring HTTP status page handler, allowing to pass in a custom template similarly to what I did in #163 for memberlist.

I strongly recommend checking the commits (and their descriptions) individually to make the review easier.

Which issue(s) this PR fixes:

None

Checklist

  • Tests updated
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

colega added 6 commits May 6, 2022 13:29
I don't want to give http handler the access to perform random CAS on
the rings, especially since I want to export the http handler and that
would mean that I need to export CasRing(), and that's something I want
to avoid.

Signed-off-by: Oleg Zaytsev <[email protected]>
Since this operation returns a Desc, IMO that's a better name.

Signed-off-by: Oleg Zaytsev <[email protected]>
I don't want to start the file with that, since it's not something
needed for users coming to see how to use this.

Signed-off-by: Oleg Zaytsev <[email protected]>
It also allows configuring the template used to be rendered.

Signed-off-by: Oleg Zaytsev <[email protected]>
@colega colega force-pushed the customizable-ring-http-handler branch from b67e611 to 69bc4ee Compare May 6, 2022 11:32
Signed-off-by: Oleg Zaytsev <[email protected]>
@colega colega requested a review from pracucci May 6, 2022 11:33
Instead of asking for the timeout when instantiating the http status
handler, move the logic of determining that to the rings implemeantion.

This makes it easier to instantiate in downstream projects (we don't
need to lookup the config).

Signed-off-by: Oleg Zaytsev <[email protected]>
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Easy! 👍

@colega colega marked this pull request as draft May 6, 2022 12:31
@colega
Copy link
Contributor Author

colega commented May 16, 2022

(I moved this to draft as I want to make sure first that it's usable in Mimir, I found some issues using it in the services where the ring isn't initialised until service has started)

@CLAassistant
Copy link

CLAassistant commented Jun 15, 2022

CLA assistant check
All committers have signed the CLA.

@colega colega closed this Dec 5, 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

Successfully merging this pull request may close these issues.

3 participants