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

Upload-process uses combined-upload endpoint #551

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

tony-codecov
Copy link

@tony-codecov tony-codecov commented Nov 6, 2024

This PR modifies the upload-process command to use a new combined upload approach that consolidates the create commit, create report, and upload steps into a single network call when possible. This optimization reduces network overhead by combining what were previously three separate API calls into one.

  • Added new combined_upload_logic function that handles all three steps (create commit, create report, and upload) in one operation
  • Modified upload-process command to use the combined upload logic by default for coverage reports when not using legacy uploader

For legacy uploads, upload-process still behaves the same as previously.

Depends on codecov/codecov-api#962

Closes codecov/engineering-team#2538

Copy link

github-actions bot commented Nov 6, 2024

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

Copy link

codecov bot commented Nov 7, 2024

❌ 5 Tests Failed:

Tests completed Failed Passed Skipped
3530 5 3525 0
View the top 3 failed tests by shortest run time
api.temp.calculator.test_calculator::test_divide
Stack Traces | 0.001s run time
def
                test_divide():
                > assert Calculator.divide(1, 2) == 0.5
                E assert 1.0 == 0.5
                E + where 1.0 = <function Calculator.divide at 0x104c9eb90>(1, 2)
                E + where <function Calculator.divide at 0x104c9eb90> = Calculator.divide
                .../temp/calculator/test_calculator.py:30: AssertionError
api.temp.calculator.test_calculator::test_divide
Stack Traces | 0.001s run time
def
                test_divide():
                > assert Calculator.divide(1, 2) == 0.5
                E assert 1.0 == 0.5
                E + where 1.0 = <function Calculator.divide at 0x104c9eb90>(1, 2)
                E + where <function Calculator.divide at 0x104c9eb90> = Calculator.divide
                .../temp/calculator/test_calculator.py:30: AssertionError
api.temp.calculator.test_calculator::test_divide
Stack Traces | 0.001s run time
def
                test_divide():
                > assert Calculator.divide(1, 2) == 0.5
                E assert 1.0 == 0.5
                E + where 1.0 = <function Calculator.divide at 0x104c9eb90>(1, 2)
                E + where <function Calculator.divide at 0x104c9eb90> = Calculator.divide
                .../temp/calculator/test_calculator.py:30: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
Got feedback? Let us know on Github

@tony-codecov tony-codecov marked this pull request as ready for review November 8, 2024 18:42
@tony-codecov tony-codecov changed the title Single endpoint coverage command initial implementation Upload-process uses combined-upload endpoint Nov 8, 2024
@tony-codecov tony-codecov requested a review from a team November 8, 2024 18:54
Swatinem
Swatinem previously approved these changes Nov 11, 2024
codecov_cli/services/upload/upload_sender.py Outdated Show resolved Hide resolved
Swatinem
Swatinem previously approved these changes Nov 12, 2024
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.

[CLI] Move upload-process command to use single upload endpoint
2 participants