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

[CLI/API] When the CLI command empty-upload --force is used on a repo with carry forward flags, the notifications fail #2365

Closed
drazisil-codecov opened this issue Aug 21, 2024 · 67 comments
Assignees
Labels
Area: Report Processing Issues with report processing Area: Report Upload Issues with pre-ingest report uploading bug Something isn't working High High Priority Issues (to be fixed within 2 sprints)

Comments

@drazisil-codecov
Copy link

drazisil-codecov commented Aug 21, 2024

When the CLI command empty-upload --force is used on a repo with carry forward flags, Notifications fail due to the notifications task being called by API before the report has a chance to be populated with the report totals

Image

@drazisil-codecov drazisil-codecov added Area: Report Processing Issues with report processing Area: Report Upload Issues with pre-ingest report uploading bug Something isn't working Medium Medium Priority Issues (to be fixed or re-evaluated in 3 months labels Aug 21, 2024
@drazisil-codecov drazisil-codecov changed the title [CLI/API] When the CLI command empty-upload is used, followed by send-notifications, the process fails [CLI/API] When the CLI command empty-upload --force is used on a repo with carry forward flags, the notification fail Aug 21, 2024
@drazisil-codecov drazisil-codecov changed the title [CLI/API] When the CLI command empty-upload --force is used on a repo with carry forward flags, the notification fail [CLI/API] When the CLI command empty-upload --force is used on a repo with carry forward flags, the notifications fail Aug 21, 2024
@eliatcodecov
Copy link

Note that --force isn't documented and is only discoverable in code. At a minimum we should document this and state that it doesn't work with carryforward flags.

More generally, we should align on why this feature exists, if it should exist, and if so how to document it. It appears this was just implemented as a convenience method to send notifications without necessarily having to call send notification manually. Do we even to have this flag?

@erickpeirson
Copy link

erickpeirson commented Sep 10, 2024

I believe that we are impacted by this issue. We use empty-upload to ensure that we trigger a CodeCov report regardless of whether CI has generated coverage data, so that we can globally require the codecov/patch status to merge pull requests.

We use carry-forward flags extensively.

Note that without the --force flag, calls to empty-upload fail with:

info - 2024-09-10 18:24:39,260 -- Process Empty Upload complete
debug - 2024-09-10 18:24:39,260 -- Empty Upload result --- {"result": "RequestResult(error=RequestError(code='HTTP Error 404', params={}, description='{\"detail\":\"Unable to get pull request for commit: 19cc963c9bda826fe1a8e4d2e00bf8b05cede023\"}'), warnings=[], status_code=404, text='{\"detail\":\"Unable to get pull request for commit: 19cc963c9bda826fe1a8e4d2e00bf8b05cede023\"}')"}
error - 2024-09-10 18:24:39,260 -- Empty Upload failed: {"detail":"Unable to get pull request for commit: 19cc963c9bda826fe1a8e4d2e00bf8b05cede023"}

What is the correct way to ensure that CodeCov always generates a report on PRs in a project that relies on carry-forward flags?

@drazisil-codecov
Copy link
Author

Hi @erickpeirson,

Can you please try calling create-commit before empty-upload?

You might need to call create-report in between those two, but I think empty-upload handles the same thing.

You should then get your notifications after we generate the "empty" report with all the carried forward coverage data.

@erickpeirson
Copy link

Can you please try calling create-commit before empty-upload?

Yes, we are already doing this today

You might need to call create-report in between those two, but I think empty-upload handles the same thing.

This appears to have done the trick!

@drazisil-codecov
Copy link
Author

(Forgot this was was a bug ticket and not a customer one 🤦‍♀

@erickpeirson
Copy link

@drazisil-codecov bad news.... this is now more broken than before. This is having a pretty big impact on CI for us.


debug - 2024-09-12 20:15:37,714 -- Using ci service from provider name: BuildKite
--
  | debug - 2024-09-12 20:15:37,715 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
  | debug - 2024-09-12 20:15:37,717 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
  | debug - 2024-09-12 20:15:37,718 -- Loading config from /workdir/codecov.yml
  | debug - 2024-09-12 20:15:37,735 -- Starting create commit process --- {"commit_sha": "aa202051e59e09d04ca3f2e8b08069f6ba9a6148", "parent_sha": null, "pr": null, "branch": "main", "slug": "datagravity-ai/dquality", "token": "6******************", "service": "github", "enterprise_url": null}
  | info - 2024-09-12 20:15:37,900 -- Process Commit creating complete
  | debug - 2024-09-12 20:15:37,900 -- Commit creating result --- {"result": "RequestResult(error=None, warnings=[], status_code=201, text='{\"message\":\"...\",\"timestamp\":\"2024-09-12T15:53:32Z\",\"ci_passed\":false,\"state\":\"complete\",\"repository\":{\"name\":\"dquality\",\"is_private\":true,\"active\":true,\"language\":\"python\",\"yaml\":{\"ignore\":[\"^ai_lib/sql_fixer.*\",\"^ai_lib/sql_generator.*\",\"^www/manage.py.*\",\"(?s:www/site_apps/main/migrations/[^\\\\\\\\/]*_auto_[^\\\\\\\\/]*\\\\\\\\.py)\\\\\\\\Z\",\"^www/tests.*\"],\"comment\":{\"layout\":\"reach, diff, components, files\",\"show_carryforward_flags\":false},\"flag_management\":{\"default_rules\":{\"statuses\":[{\"base\":\"auto\",\"type\":\"project\",\"target\":\"auto\",\"threshold\":1.0,\"if_ci_failed\":\"error\"},{\"type\":\"patch\",\"target\":100.0,\"if_ci_failed\":\"error\"}],\"carryforward\":true},\"individual_flags\":[{\"name\":\"dquality\",\"paths\":[\".*\"],\"carryforward\":true},{\"name\":\"backtests\",\"paths\":[\".*\"],\"carryforward\":true},{\"name\":\"shared\",\"paths\":[\"shared/.*\"],\"carryforward\":true},{\"name\":\"web-backend\",\"paths\":[\"www/.*\"],\"carryforward\":true}]},\"component_management\":{\"individual_components\":[{\"name\":\"Data warehouse connectors\",\"paths\":[\"^dquality/db.*\"],\"component_id\":\"dbconnectors\"},{\"name\":\"Data quality core\",\"paths\":[\"^dquality.*\"],\"component_id\":\"dquality\"},{\"name\":\"Backend web app\",\"paths\":[\"^www.*\"],\"component_id\":\"backend\"},{\"name\":\"Shared library\",\"paths\":[\"^shared.*\"],\"component_id\":\"shared\"},{\"name\":\"CLI apps\",\"paths\":[\"^apps.*\"],\"component_id\":\"apps\"}]}}},\"author\":{\"avatar_url\":\"https://avatars0.githubusercontent.com/u/6236330?v=3&s=55\",\"service\":\"github\",\"username\":\"kfoxb\",\"name\":\"Kyle Bradford\",\"ownerid\":3670035},\"commitid\":\"aa202051e59e09d04ca3f2e8b08069f6ba9a6148\",\"parent_commit_id\":\"309fd0073938a8a90743e84fb53e14b1161013c6\",\"pullid\":null,\"branch\":\"main\"}')"}
  | debug - 2024-09-12 20:15:38,179 -- Using ci service from provider name: BuildKite
  | debug - 2024-09-12 20:15:38,181 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
  | debug - 2024-09-12 20:15:38,182 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
  | debug - 2024-09-12 20:15:38,183 -- Loading config from /workdir/codecov.yml
  | debug - 2024-09-12 20:15:38,193 -- Starting create report process --- {"commit_sha": "aa202051e59e09d04ca3f2e8b08069f6ba9a6148", "code": "default", "slug": "datagravity-ai/dquality", "service": "github", "enterprise_url": null, "token": "6******************"}
  | info - 2024-09-12 20:15:38,320 -- Process Report creating complete
  | debug - 2024-09-12 20:15:38,320 -- Report creating result --- {"result": "RequestResult(error=None, warnings=[], status_code=201, text='{\"code\":null}')"}
  | info - 2024-09-12 20:15:38,320 -- Finished creating report successfully --- {"response": "{\"code\":null}"}
  | debug - 2024-09-12 20:15:38,598 -- Using ci service from provider name: BuildKite
  | debug - 2024-09-12 20:15:38,600 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
  | debug - 2024-09-12 20:15:38,601 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
  | debug - 2024-09-12 20:15:38,602 -- Loading config from /workdir/codecov.yml
  | debug - 2024-09-12 20:15:38,612 -- Starting empty upload process --- {"commit_sha": "aa202051e59e09d04ca3f2e8b08069f6ba9a6148", "slug": "datagravity-ai/dquality", "token": "6******************", "service": "github", "enterprise_url": null, "fail_on_error": false}
  | warning - 2024-09-12 20:15:38,765 -- Response status code was 500. --- {"retry": 0}
  | warning - 2024-09-12 20:15:38,765 -- Request failed. Retrying --- {"retry": 0}
  | warning - 2024-09-12 20:15:39,414 -- Response status code was 500. --- {"retry": 1}
  | warning - 2024-09-12 20:15:39,414 -- Request failed. Retrying --- {"retry": 1}
  | warning - 2024-09-12 20:15:40,588 -- Response status code was 500. --- {"retry": 2}
  | warning - 2024-09-12 20:15:40,588 -- Request failed. Retrying --- {"retry": 2}
  | Traceback (most recent call last):
  | File "/usr/local/bin/codecovcli", line 8, in <module>
  | sys.exit(run())
  | File "/usr/local/lib/python3.10/site-packages/codecov_cli/main.py", line 81, in run
  | cli(obj={})
  | File "/usr/local/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
  | return self.main(*args, **kwargs)
  | File "/usr/local/lib/python3.10/site-packages/click/core.py", line 1078, in main
  | rv = self.invoke(ctx)
  | File "/usr/local/lib/python3.10/site-packages/click/core.py", line 1688, in invoke
  | return _process_result(sub_ctx.command.invoke(sub_ctx))
  | File "/usr/local/lib/python3.10/site-packages/click/core.py", line 1434, in invoke
  | return ctx.invoke(self.callback, **ctx.params)
  | File "/usr/local/lib/python3.10/site-packages/click/core.py", line 783, in invoke
  | return __callback(*args, **kwargs)
  | File "/usr/local/lib/python3.10/site-packages/click/decorators.py", line 33, in new_func
  | return f(get_current_context(), *args, **kwargs)
  | File "/usr/local/lib/python3.10/site-packages/codecov_cli/commands/empty_upload.py", line 41, in empty_upload
  | return empty_upload_logic(
  | File "/usr/local/lib/python3.10/site-packages/codecov_cli/services/empty_upload/__init__.py", line 22, in empty_upload_logic
  | sending_result = send_post_request(
  | File "/usr/local/lib/python3.10/site-packages/codecov_cli/helpers/request.py", line 82, in wrapper
  | raise Exception("Request failed after too many retries")
  | Exception: Request failed after too many retries


@drazisil-codecov
Copy link
Author

Hi @erickpeirson ,

We were trying to fix it last night, sorry that it broke. This was with --force, or no?

@erickpeirson
Copy link

Hi @erickpeirson ,

We were trying to fix it last night, sorry that it broke. This was with --force, or no?

Without --force

@drazisil-codecov
Copy link
Author

Is it still broken? runs a test to see

@drazisil-codecov
Copy link
Author

drazisil-codecov commented Sep 13, 2024

Fix PR for the 500 (not --force) is running CI.

@drazisil-codecov
Copy link
Author

@erickpeirson the 500 is fixed.

@rohan-at-sentry rohan-at-sentry added High High Priority Issues (to be fixed within 2 sprints) and removed Medium Medium Priority Issues (to be fixed or re-evaluated in 3 months labels Sep 13, 2024
@codecov-hooky codecov-hooky bot closed this as completed Sep 13, 2024
@codecov-hooky codecov-hooky bot closed this as completed Sep 13, 2024
@rohan-at-sentry rohan-at-sentry removed the High High Priority Issues (to be fixed within 2 sprints) label Sep 16, 2024
@codecov-hooky codecov-hooky bot closed this as completed Sep 16, 2024
@drazisil-codecov
Copy link
Author

Hooky, stop closing this.

@giovanni-guidini
Copy link

Is this quote still accurate? (from the 1st comment)
That is, is that the bug?

When the CLI command empty-upload --force is used on a repo with carry forward flags, Notifications fail due to the notifications task being called by API before the report has a chance to be populated with the report totals

@drazisil-codecov
Copy link
Author

@Ryang20718 The logs you pasted do not show an error. Ryang20718/cflag#16 (comment) looks correct. What issue are you seeing?

@Ryang20718
Copy link

@drazisil-codecov

Image

I'm expecting to see these status checks as "passed" rather than empty coverage based on docs. Am I misunderstanding?

@drazisil-codecov
Copy link
Author

@Ryang20718 You are correct, those should be pass if this was working correctly. @Swatinem please see above for a public example where --force is not returning successful checks.

@Ryang20718
Copy link

@drazisil-codecov @Swatinem let me know if there's any other info I can provide or if there's any way I can help push this over the finish line!

Dayjob uses an internal job that polls the codecov api to do similar functionality of those status checks which is reinventing the wheel, but we do this purely because of this lack of bypass

@drazisil-codecov
Copy link
Author

Hi @Ryang20718

We think we identified the problem and are waiting for the engineer to commit a PR. Shouldn't be too much longer. Sorry this was a pain to locate.

@drazisil-codecov
Copy link
Author

@Ryang20718 https://github.com/codecov/shelter/pull/215 should have fixed it. Can you please test? 🤞

@jhecking
Copy link

jhecking commented Nov 7, 2024

@drazisil-codecov is Ryang20718's issue the same issue that is affecting us? Or are those two separate issues?

@drazisil-codecov
Copy link
Author

@jhecking , sorry. Yes, this should be fixed for you now as well, I believe it was the same issue. Please remember to use --force for a passing check.

@Ryang20718
Copy link

Ryang20718 commented Nov 7, 2024

@drazisil-codecov

This is still failing (status checks aren't bypassed and just never reported). the changes that fix the issue are server side right?

Image
Image
Ryang20718/cflag#16

@jhecking
Copy link

jhecking commented Nov 8, 2024

It seems to finally work for us! But after many previous disappointments, I'll keep an eye on this to make sure it's not just a fluke but really a permanent solution.

Image

@drazisil-codecov
Copy link
Author

Hi @Ryang20718 , yes, the issue we found was the --force was not being passed though the entire flow to the place where it is checked for the check to pass or fail.

I'm surprised it worked for Jan and not you. That's encouraging, at least. 🤔

@Swatinem This error was AttributeError: 'NoneType' object has no attribute 'totals'" and also missing "sessions"
SHA is a606c4314528dc41381f78e2476f110d5b4ef5c9

@drazisil-codecov
Copy link
Author

@jhecking That's what I love to see! I completely understand your concerns. Please let me know if it stops again.

@Ryang20718
Copy link

Ryang20718 commented Nov 8, 2024

@drazisil-codecov I created a new PR in my codebase and added this new empty upload feature and it does indeed work. I think rerunning my old PR just doesn't seem to work.

I think there's an edge case where this API doesn't work.

  1. upload legimitate coverage, get results in PR for status checks
  2. retrigger PR, but now modify codecov to upload empty report
  3. status checks remain at expecting

example Ryang20718/cflag#24

@drazisil-codecov
Copy link
Author

@Swatinem ^ <3

@Ryang20718
Copy link

Ryang20718 commented Nov 15, 2024

Appreciate yall driving these issues down!

please let me know if codecov/worker#881 is supposed to fix this issue?

If not, please disregard this message below (still seems to be stuck on waiting to be reported) (I just re-triggered)
Image

@drazisil-codecov
Copy link
Author

drazisil-codecov commented Nov 15, 2024

Hi @Ryang20718 It looks like checks were sent to repos/Ryang20718/cflag/check-runs/33020188747 (API endpoint). I'm not having success locating where that maps to. GitHub did tell up 200, though. 🤔

edit:af185d140ab573953fcaa0bcdd13a7a02b505e0d is the correct head SHA, right?

@Ryang20718
Copy link

@drazisil-codecov af185d140ab573953fcaa0bcdd13a7a02b505e0d is the correct sha. this is the run https://github.com/Ryang20718/cflag/actions/runs/11749458250/job/33020182406?pr=24

PR link Ryang20718/cflag#24

@drazisil-codecov
Copy link
Author

@Ryang20718 Yes, I think the error that af185d140ab573953fcaa0bcdd13a7a02b505e0d had is fixed by codecov/worker#894

Can you test again? (cc: @Swatinem

@Ryang20718
Copy link

Ryang20718 commented Nov 15, 2024

@drazisil-codecov re-triggered with the same commit Image

Link to logs https://github.com/Ryang20718/cflag/actions/runs/11749458250/job/33058871228?pr=24

info - 2024-11-15 18:24:34,635 -- ci service found: github-actions
debug - 2024-[11](https://github.com/Ryang20718/cflag/actions/runs/11749458250/job/33058871228?pr=24#step:3:12)-15 18:24:34,638 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
debug - 2024-11-15 18:24:34,641 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
debug - 2024-11-15 18:24:34,643 -- Loading config from /home/runner/work/cflag/cflag/.codecov.yml
debug - 2024-11-15 18:24:34,653 -- Starting create commit process --- {"verbose": true, "auto_load_params_from": null, "codecov_yml_path": null, "enterprise_url": null, "version": "cli-0.8.0", "command": "create-commit", "parent_sha": null, "pull_request_number": "24", "branch": "testing_empty", "commit_sha": "af185d140ab573953fcaa0bcdd13a7a02b505e0d", "fail_on_error": false, "git_service": "github", "slug": "Ryang20718/cflag"}
info - 2024-11-15 18:24:35,186 -- Process Commit creating complete
debug - 2024-11-15 18:24:35,186 -- Commit creating result --- {"result": "RequestResult(error=None, warnings=[], status_code=202, text='{\"status\":\"queued\"}\\n')"}
info - 2024-11-15 18:24:35,927 -- ci service found: github-actions
debug - 2024-11-15 18:24:35,930 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
debug - 2024-11-15 18:24:35,933 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
debug - 2024-11-15 18:24:35,936 -- Loading config from /home/runner/work/cflag/cflag/.codecov.yml
debug - 2024-11-15 18:24:35,945 -- Starting create report process --- {"verbose": true, "auto_load_params_from": null, "codecov_yml_path": null, "enterprise_url": null, "version": "cli-0.8.0", "command": "create-report", "code": "default", "pull_request_number": "24", "commit_sha": "af185d140ab573953fcaa0bcdd13a7a02b505e0d", "fail_on_error": false, "git_service": "github", "slug": "Ryang20718/cflag"}
info - 2024-11-15 18:24:36,377 -- Process Report creating complete
debug - 2024-11-15 18:24:36,378 -- Report creating result --- {"result": "RequestResult(error=None, warnings=[], status_code=202, text='{\"status\":\"queued\"}\\n')"}
info - 2024-11-15 18:24:36,378 -- Finished creating report successfully --- {"response": "{\"status\":\"queued\"}\n"}
info - 2024-11-15 18:24:37,110 -- ci service found: github-actions
debug - 2024-11-15 18:24:37,113 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
debug - 2024-11-15 18:24:37,115 -- versioning system found: <class 'codecov_cli.helpers.versioning_systems.GitVersioningSystem'>
debug - 2024-11-15 18:24:37,118 -- Loading config from /home/runner/work/cflag/cflag/.codecov.yml
debug - 2024-11-15 18:24:37,[12](https://github.com/Ryang20718/cflag/actions/runs/11749458250/job/33058871228?pr=24#step:3:13)8 -- Starting empty upload process --- {"verbose": true, "auto_load_params_from": null, "codecov_yml_path": null, "enterprise_url": null, "version": "cli-0.8.0", "command": "empty-upload", "force": true, "commit_sha": "af185d140ab573953fcaa0bcdd[13](https://github.com/Ryang20718/cflag/actions/runs/11749458250/job/33058871228?pr=24#step:3:14)a7a02b505e0d", "fail_on_error": false, "git_service": "github", "slug": "Ryang20718/cflag"}
info - 2024-11-15 18:24:37,579 -- Process Empty Upload complete
debug - 2024-11-15 18:24:37,579 -- Empty Upload result --- {"result": "RequestResult(error=None, warnings=[], status_code=202, text='{\"status\":\"queued\"}\\n')"}

status checks are still stuck at waiting to be reported after rerunning the empty upload

@drazisil-codecov
Copy link
Author

I've asked @suejung-sentry to take a look, as it's really late for Swatinem. It's getting kinda late, so we may have to pick this back up Monday, @Ryang20718 . I appreciate the patience and cooperation.

@drazisil-codecov
Copy link
Author

Hi @Ryang20718 Looks like we are sending checks on the merge commit https://github.com/Ryang20718/cflag/runs/33058885826

Ryang20718/cflag@af185d1

We can see the checks here Ryang20718/cflag#15 as well, when we expand the details.

Did you rerun the CI job?

Ideally we should have sent checks on Ryang20718/cflag@4abb84a as well, which it looks like we did. Is this finally behaving?

@Ryang20718
Copy link

@drazisil-codecov that's a different run which works for the happy path. The bug occurs when

  1. upload legimitate coverage, get results in PR for status checks
  2. retrigger PR, but now modify codecov to upload empty report
  3. status checks remain at expecting

The run that I am mentioning is attempt # 3 https://github.com/Ryang20718/cflag/actions/runs/11749458250/attempts/3?pr=24

on pull request # 24

I re-triggered another run just now https://github.com/Ryang20718/cflag/actions/runs/11749458250/job/33151080963?pr=24 (1 minute ago.

@drazisil-codecov
Copy link
Author

Hi @Ryang20718

retrigger PR, but now modify codecov to upload empty report

I think I'm confused here. How are you doing this? Any changes should be on a new SHA, are they not in this case?

@Ryang20718
Copy link

@drazisil-codecov

github actions allows the ability to rerun workflows Image

I also made a new commit Ryang20718/cflag@3581250 to retrigger CI to upload empty coverage, but that also has the same error

@drazisil-codecov
Copy link
Author

Hi @Ryang20718

I apologize, what i was unclear on was if you were saying you edut the workflow when yoy re-can it. The checks for https://github.com/Ryang20718/cflag/actions/runs/11897300171/job/33151232017#step:3:15 posted on Ryang20718/cflag@39cd41c, which is the SHA they were uploaded under.

Ypu may need to use the --sha flag https://docs.codecov.com/docs/cli-options#do-upload to tell Codedov what the correct SHA is, since it appears our autodetection is not picking the expected one.

@Ryang20718
Copy link

@drazisil-codecov That was the issue 👍 thanks!

@drazisil-codecov
Copy link
Author

It is my hope that any further issues should be resolved by these two PRs

It's not clear if these two PRs have deployed yet or not, since we are in the Sentry LaunchWeek code freeze. I will revisit this Monday and see if any further issues have been reported before I mark this done.

Please let me know if anyone is still seeing this (cc: @thomasrockhu-codecov

@drazisil-codecov
Copy link
Author

@ all

There are quite a few customers on this thread. Before I close it, is anyone still seeing issues related to this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Report Processing Issues with report processing Area: Report Upload Issues with pre-ingest report uploading bug Something isn't working High High Priority Issues (to be fixed within 2 sprints)
Projects
None yet
Development

No branches or pull requests

9 participants