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

refactor: deprecate AuthToken for non-OG pyroscope cloud #125

Merged
merged 3 commits into from
Sep 9, 2024

Conversation

marcsanmi
Copy link
Contributor

@marcsanmi marcsanmi commented Sep 9, 2024

Closes #124

This PR deprecates the use of AuthToken for non-OG Pyroscope cloud.

Changes:

  • Added deprecation notices to AuthToken in Config structs
  • Updated authentication logic in uploadProfile function
  • Added warning for deprecated AuthToken usage with non-OG Pyroscope cloud
  • Maintained AuthToken support for OG Pyroscope cloud servers
  • Ensured BasicAuth is used when provided, regardless of server type

@marcsanmi marcsanmi requested a review from a team as a code owner September 9, 2024 07:29
@CLAassistant
Copy link

CLAassistant commented Sep 9, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

upstream/remote/remote.go Outdated Show resolved Hide resolved
@korniltsev
Copy link
Collaborator

nit: maybe add a test if possible and you think it may be useful, but I don't think it is required

@marcsanmi
Copy link
Contributor Author

marcsanmi commented Sep 9, 2024

I added tests for the uploadProfile function in the remote package, covering various authentication scenarios. I also Introduced a new internal/testutil package with a TestLogger for shared testing utilities, following a similar approach used in projects like Prometheus.

Note that some functions and packages in the SDK still lack tests. We should keep adding them as we continue working on it.

@korniltsev
Copy link
Collaborator

Awesome

@marcsanmi marcsanmi enabled auto-merge (squash) September 9, 2024 13:38
@marcsanmi marcsanmi enabled auto-merge (squash) September 9, 2024 13:39
@korniltsev korniltsev merged commit 3551b4c into main Sep 9, 2024
8 checks passed
@korniltsev korniltsev deleted the deprecate-auth-token branch September 9, 2024 18:08
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.

Deprecate AuthToken
3 participants