Skip to content

Conversation

@panbingkun
Copy link
Contributor

@panbingkun panbingkun commented Jan 15, 2025

What changes were proposed in this pull request?

The pr aims to let CODECOV_TOKEN transfer from build_coverage.yml/build_main.yml to build_and_test.yml.

Why are the changes needed?

Currently, codecov/codecov-action is unable to obtain CODECOV_TOKEN.
I checked and it seems that the usage below is similar to ours.
https://stackoverflow.com/questions/78298827/why-is-codecov-upload-step-in-github-actions-not-finding-the-token
Let's continue to try it out.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Manually check.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the INFRA label Jan 15, 2025
@panbingkun
Copy link
Contributor Author

panbingkun commented Jan 15, 2025

cc @dongjoon-hyun @HyukjinKwon @zhengruifeng @LuciferYang
Could you take a look? I want to try this approach.
Thanks!

@panbingkun panbingkun marked this pull request as ready for review January 15, 2025 12:06
uses: codecov/codecov-action@v5
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
CODECOV_TOKEN: ${{ inputs.codecov_token }}
Copy link
Contributor

Choose a reason for hiding this comment

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

After this change, will inputs.codecov_token be in plaintext?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the current approach, token is no longer plaintext.

@dongjoon-hyun
Copy link
Member

Just want to check. According to the commit log and CI result, this isn't ready yet. Did I understand correctly, @panbingkun ?

@panbingkun
Copy link
Contributor Author

Just want to check. According to the commit log and CI result, this isn't ready yet. Did I understand correctly, @panbingkun ?

  • Yeah, the first version should be functional, but there may be security issues;
  • In the subsequent updates, I encountered an issue as follows:
    when I add secrets.codecov_token to build_main.yml
image the following error will occur image @HyukjinKwon, do you know what the possible reasons could be?
  • And it seems that notify_test_workflow.yml cannot be debugged, I am trying to find a solution to it.

@panbingkun
Copy link
Contributor Author

panbingkun commented Jan 16, 2025

@dongjoon-hyun
I think I've already taken care of this issue.
(BTW, I created a new mock repo myself to conduct validation testing and debugging).

@panbingkun panbingkun changed the title [SPARK-50633][FOLLOWUP] Let CODECOV_TOKEN transfer from build_coverage.yml to build_and_test.yml [SPARK-50633][FOLLOWUP] Let CODECOV_TOKEN transfer to build_and_test.yml Jan 16, 2025
@panbingkun
Copy link
Contributor Author

panbingkun commented Jan 16, 2025

image https://github.com/panbingkun/github-actions/actions/runs/12804808017/job/35700210844 image image image image

secrets:
codecov_token:
description: The upload token of codecov.
required: false
Copy link
Contributor

Choose a reason for hiding this comment

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

So the previous issue was whether codecov_token shouldn't specify default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

type and default cannot be set in secrets, otherwise an error will be reported.

@LuciferYang
Copy link
Contributor

hmm... can we make a clean one?

@panbingkun
Copy link
Contributor Author

panbingkun commented Jan 16, 2025

hmm... can we make a clean one?

Okay

#49527.

@panbingkun
Copy link
Contributor Author

closing in favor of #49527

@panbingkun panbingkun closed this Jan 16, 2025
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