-
Notifications
You must be signed in to change notification settings - Fork 1
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
Comments
empty-upload
is used, followed by send-notifications
, the process failsempty-upload --force
is used on a repo with carry forward flags, the notification fail
empty-upload --force
is used on a repo with carry forward flags, the notification failempty-upload --force
is used on a repo with carry forward flags, the notifications fail
Note that 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? |
I believe that we are impacted by this issue. We use We use carry-forward flags extensively. Note that without the
What is the correct way to ensure that CodeCov always generates a report on PRs in a project that relies on carry-forward flags? |
Hi @erickpeirson, Can you please try calling You might need to call You should then get your notifications after we generate the "empty" report with all the carried forward coverage data. |
Yes, we are already doing this today
This appears to have done the trick! |
(Forgot this was was a bug ticket and not a customer one 🤦♀ |
@drazisil-codecov bad news.... this is now more broken than before. This is having a pretty big impact on CI for us.
|
Hi @erickpeirson , We were trying to fix it last night, sorry that it broke. This was with |
Without |
Is it still broken? runs a test to see |
Fix PR for the 500 (not |
@erickpeirson the 500 is fixed. |
Hooky, stop closing this. |
Is this quote still accurate? (from the 1st comment)
|
@Ryang20718 The logs you pasted do not show an error. Ryang20718/cflag#16 (comment) looks correct. What issue are you seeing? |
I'm expecting to see these status checks as "passed" rather than empty coverage based on docs. Am I misunderstanding? |
@Ryang20718 You are correct, those should be pass if this was working correctly. @Swatinem please see above for a public example where |
@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 |
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. |
@Ryang20718 https://github.com/codecov/shelter/pull/215 should have fixed it. Can you please test? 🤞 |
@drazisil-codecov is Ryang20718's issue the same issue that is affecting us? Or are those two separate issues? |
@jhecking , sorry. Yes, this should be fixed for you now as well, I believe it was the same issue. Please remember to use |
This is still failing (status checks aren't bypassed and just never reported). the changes that fix the issue are server side right? |
Hi @Ryang20718 , yes, the issue we found was the I'm surprised it worked for Jan and not you. That's encouraging, at least. 🤔 @Swatinem This error was |
@jhecking That's what I love to see! I completely understand your concerns. Please let me know if it stops again. |
@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.
example Ryang20718/cflag#24 |
@Swatinem ^ <3 |
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) |
Hi @Ryang20718 It looks like checks were sent to edit: |
@drazisil-codecov PR link Ryang20718/cflag#24 |
@Ryang20718 Yes, I think the error that Can you test again? (cc: @Swatinem |
@drazisil-codecov re-triggered with the same commit Link to logs https://github.com/Ryang20718/cflag/actions/runs/11749458250/job/33058871228?pr=24
status checks are still stuck at waiting to be reported after rerunning the empty upload |
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. |
Hi @Ryang20718 Looks like we are sending checks on the merge commit https://github.com/Ryang20718/cflag/runs/33058885826 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? |
@drazisil-codecov that's a different run which works for the happy path. The bug occurs when
The run that I am mentioning is attempt # 3 https://github.com/Ryang20718/cflag/actions/runs/11749458250/attempts/3?pr=24 I re-triggered another run just now https://github.com/Ryang20718/cflag/actions/runs/11749458250/job/33151080963?pr=24 (1 minute ago. |
Hi @Ryang20718
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? |
github actions allows the ability to rerun workflows I also made a new commit Ryang20718/cflag@3581250 to retrigger CI to upload empty coverage, but that also has the same error |
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 |
@drazisil-codecov That was the issue 👍 thanks! |
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 |
@ all There are quite a few customers on this thread. Before I close it, is anyone still seeing issues related to this? |
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 totalsThe text was updated successfully, but these errors were encountered: