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

update codecov process #3336

Merged
merged 2 commits into from
Jan 19, 2025
Merged

update codecov process #3336

merged 2 commits into from
Jan 19, 2025

Conversation

hirosassa
Copy link
Contributor

@hirosassa hirosassa commented Jan 18, 2025

Description

In this PR, I replaced codecov process to official github action.

Motivation and Context

codecov command is currently deprecated.
ref: https://docs.codecov.com/docs/about-the-codecov-bash-uploader

Have you tested this? If so, how?

Fixed CI worked.

@hirosassa hirosassa requested review from dlstadther and a team as code owners January 18, 2025 08:37
@dlstadther
Copy link
Collaborator

Take a look at the Github Action results. Failures with the codecov changes.

@hirosassa
Copy link
Contributor Author

@dlstadther Thanks for your comment! Could you please review this PR?

@hirosassa hirosassa mentioned this pull request Jan 18, 2025
@dlstadther
Copy link
Collaborator

CI execution looks good and can confirm that codecov received the test coverage results. However, unrelated to your change (i think), it looks like CodeCov is complaining about the validity of the luigi conf yaml file. Though, the yaml indentation that codecov reports looks different than the luigi/codecov.yml.

https://app.codecov.io/github/spotify/luigi/commit/955ae591ab201e270552d002723a08534a31d8ec

Copy link
Collaborator

@dlstadther dlstadther left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for this contribution and for your quick and effective fixes!

@dlstadther dlstadther merged commit 879b225 into spotify:master Jan 19, 2025
45 checks passed
@hirosassa hirosassa deleted the update-codecov branch January 19, 2025 02:07
@dlstadther
Copy link
Collaborator

dlstadther commented Jan 19, 2025

The master build is now failing because codecov's upload is requiring a token for protected branches...

debug - 2025-01-19 01:49:47,309 -- Upload result --- {"result": "RequestResult(error=RequestError(code='HTTP Error 400', params={}, description='{\"message\":\"Token required because branch is protected\"}\\n'), warnings=[], status_code=400, text='{\"message\":\"Token required because branch is protected\"}\\n')"}
error - 2025-01-19 01:49:47,309 -- Upload failed: {"message":"Token required because branch is protected"}

Odd how the old cli execution was able to upload coverage, but this method can't.
@hirosassa , could you look into options without the token? (or to reference an existing token if one is mentioned by the tox configuration.

Otherwise, we may need to involve a Spotify employee who has admin rights to help.

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.

2 participants