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

Set CODECOV_TOKEN for repo #62

Open
MasterOdin opened this issue Aug 21, 2024 · 7 comments
Open

Set CODECOV_TOKEN for repo #62

MasterOdin opened this issue Aug 21, 2024 · 7 comments

Comments

@MasterOdin
Copy link
Member

While #29 was closed with updating the codecov/codecov-action, it appears the repo was not yet setup with a CODECOV_TOKEN. See https://github.com/py-pdf/pypdf_table_extraction/actions/runs/10488522333/job/29051313824 as an example (latest run of the coverage workflow for main), where the upload step outputs:

Error: Codecov token not found. Please provide Codecov token with -t flag.

Further, if expand the first line of that step of the job:

Run codecov/codecov-action@v4
  with:
  env:
    pythonLocation: /opt/hostedtoolcache/Python/3.10.14/x64
    PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.10.14/x64/lib/pkgconfig
    Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.14/x64
    Python[2](https://github.com/py-pdf/pypdf_table_extraction/actions/runs/10488522333/job/29051313824#step:10:2)_ROOT_DIR: /opt/hostedtoolcache/Python/[3](https://github.com/py-pdf/pypdf_table_extraction/actions/runs/10488522333/job/29051313824#step:10:3).10.1[4](https://github.com/py-pdf/pypdf_table_extraction/actions/runs/10488522333/job/29051313824#step:10:4)/x64
    Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.10.14/x64
    LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.10.14/x64/lib
    CODECOV_TOKEN:

where we can see that no value was passed in for CODECOV_TOKEN.

As such, I think someone still needs to actually set ${{ secrets.CODECOV_TOKEN }} to a value for this to work. See https://docs.codecov.com/docs/adding-the-codecov-token for instructions on how to do this.

@stefan6419846
Copy link

In theory, the token should not really be required - at least for pypdf we are currently using the token-less approach. We only tend to occasionally hit the rate limit for tokenless uploads, but it general it seems to be running without issues.

@MasterOdin
Copy link
Member Author

MasterOdin commented Aug 21, 2024

Yeah, you either need to stick with @codecov/codecov-action@v3 and use tokenless and be fine that some percentage of uploads will never happen due to rate limits, or just setup the token approach with the upside that any coverage run within the repo will always upload. codecov/feedback#112 (comment) gives some details on why they made this change.

at least for pypdf we are currently using the token-less approach. We only tend to occasionally hit the rate limit for tokenless uploads, but it general it seems to be running without issues.

It's not though. For PRs/pushes within the repo, a token is being used to upload the results:

https://github.com/py-pdf/pypdf/blob/0c81f3cfad26ddffbfc60d0ae855118e515fad8c/.github/workflows/github-ci.yaml#L229-L233

PRs from forks of pypdf will use a tokenless approach, and you are correct that PRs will hit the rate limit and not be uploaded.

@bosd
Copy link
Collaborator

bosd commented Aug 21, 2024

Thanks for linking to that threat.

you either need to stick with @codecov/codecov-action@v3

Over there it was mentioned that V3 is no longer working.

Maybe the easiest way to handle this it use a token on the pypdf organization level.
See this comment. codecov/feedback#112 (comment)

@MasterOdin
Copy link
Member Author

MasterOdin commented Aug 21, 2024

you either need to stick with @codecov/codecov-action@v3

Over there it was mentioned that V3 is no longer working.

The v3 version still "works" in the sense that assuming you don't hit a rate limit, it will still upload the coverage statistics. It's just that you're very likely to hit the rate limit so no coverage will get uploaded, rendering it somewhat useless.

Maybe the easiest way to handle this it use a token on the pypdf organization level.
See this comment. codecov/feedback#112 (comment)

Agreed. I think though getting the token is limited to either @Lucas-C or @MartinThoma (depending on who setup Codecov for the org first I think?).

@Lucas-C
Copy link
Member

Lucas-C commented Aug 26, 2024

It think is is safer to use the latest GitHub Actions workflow:
there has been security issues in the past with CodeCov,
and it is more likely that the latest maintained GitHub Actions step will be patched in case of a new issue.

I checked on https://app.codecov.io/gh/py-pdf and it seems to me that coverage uploads already work fine:
image

Latest upload log: https://github.com/py-pdf/pypdf_table_extraction/actions/runs/10562010294/job/29259475281
(step : "Upload coverage report")

@MasterOdin
Copy link
Member Author

That upload worked as it was from a fork in a PR, so it allows tokenless upload (though still has a potential rate limit and so may fail randomly, but usually doesn't matter as it's for a PR). The "Last Updated" thing on codecov then updates for PRs as well as uploads from the repo, so it's a bit misleading. On the main page for pypdf_table_extraction, it shows on the upper right what the last analyzed commit was:

Screenshot 2024-08-29 at 12 14 44 PM

which was 38eb7d4 from 2 weeks ago.

The latest upload log for master (https://github.com/py-pdf/pypdf_table_extraction/actions/runs/10617184136/job/29429574174) shows that it's failed.

@bosd
Copy link
Collaborator

bosd commented Oct 9, 2024

which was 38eb7d4 from 2 weeks ago.

That is still the latest commit. The source code on codecov still shows an old version.

On one of the latest merge actions there still is this warning.
image
source: https://github.com/py-pdf/pypdf_table_extraction/actions/runs/11231172682/job/31220291182

Who has the powers here to setup the token?
cc: @MartinThoma

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

No branches or pull requests

4 participants