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

chore: update cortex to latest and fix refs #3295

Merged
merged 2 commits into from
Feb 4, 2021
Merged

Conversation

jtlisi
Copy link
Contributor

@jtlisi jtlisi commented Feb 4, 2021

What this PR does / why we need it:

This PR updates to the latest version of Cortex and updates the reference for the global logger since the original Global Logger was removed.

The number of LOC changes is due to the global logger being used throughout the project. The changes should be noops.

Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
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.

As Jacob mentioned, we did a couple of breaking changes in Cortex:

  • Moved logger from pkg/util to pkg/util/log (just moved, no logic change)
  • Rename ring.IngesterDesc to ring.InstanceDesc (just renamed, no logic change)

I went through the diff and changes LGTM.

Copy link
Member

@owen-d owen-d left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the help

@owen-d owen-d merged commit 923671a into master Feb 4, 2021
@owen-d owen-d deleted the 20210204_update_cortex branch February 4, 2021 16:50
cyriltovena pushed a commit to cyriltovena/loki that referenced this pull request Feb 15, 2021
* chore: update cortex to latest and fix refs

Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>

* tidy mod file

Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants