-
Notifications
You must be signed in to change notification settings - Fork 39
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
fix: accept tokenless value from branch instead of env var #475
base: main
Are you sure you want to change the base?
Conversation
❌ Failed Test Results:Completed 693 tests with View the full list of failed tests
|
4 similar comments
❌ Failed Test Results:Completed 693 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 693 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 693 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 693 tests with View the full list of failed tests
|
❌ 3 Tests Failed:
View the top 3 failed tests by shortest run time
To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard |
❌ Failed Test Results:Completed 693 tests with View the full list of failed tests
|
4 similar comments
❌ Failed Test Results:Completed 693 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 693 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 693 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 693 tests with View the full list of failed tests
|
b0dd421
to
691e159
Compare
❌ Failed Test Results:Completed 694 tests with View the full list of failed tests
|
4 similar comments
❌ Failed Test Results:Completed 694 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 694 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 694 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 694 tests with View the full list of failed tests
|
tokenless = os.environ.get("TOKENLESS") | ||
if tokenless: | ||
if not token and branch and ":" in branch: | ||
headers = None # type: ignore | ||
branch = tokenless # type: ignore | ||
logger.info("The PR is happening in a forked repo. Using tokenless upload.") | ||
else: | ||
headers = get_token_header_or_fail(token) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strictly speaking, we no longer need any of this logic anymore. we just send whatever we have for the token and the branch, and the API will return a 401 if it's not allowed
the logs may still be helpful, so maybe change to:
if not token:
if branch and ":" in branch:
logger.info("Creating a commit on an unprotected branch")
else:
logger.warning("Token is missing, but branch is missing or protected")
headers = get_token_header(token)
❌ Failed Test Results:Completed 696 tests with View the full list of failed tests
|
4 similar comments
❌ Failed Test Results:Completed 696 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 696 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 696 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 696 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 696 tests with View the full list of failed tests
|
1 similar comment
❌ Failed Test Results:Completed 696 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 696 tests with View the full list of failed tests
|
Signed-off-by: joseph-sentry <[email protected]>
Signed-off-by: joseph-sentry <[email protected]>
just pass through token and branch and let the API handle the failure if the token is undefined and the branch is protected
4250717
to
24daf24
Compare
❌ Failed Test Results:Completed 696 tests with View the full list of failed tests
|
2 similar comments
❌ Failed Test Results:Completed 696 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 696 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 698 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 698 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 698 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 698 tests with View the full list of failed tests
|
2 similar comments
❌ Failed Test Results:Completed 698 tests with View the full list of failed tests
|
❌ Failed Test Results:Completed 698 tests with View the full list of failed tests
|
depends on: codecov/codecov-action#1511
fixes: codecov/engineering-team#2065