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.

Sign up for GitHub

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

Jump to bottom

fix: don't eager merge sessions into report #336

Merged
merged 2 commits into from
Mar 18, 2024
Merged

fix: don't eager merge sessions into report #336

merged 2 commits into from
Mar 18, 2024

Commits on Mar 18, 2024

  1. fix: don't eager merge sessions into report 

    context: https://l.codecov.dev/xvGZaM

Because we eagerly merge sessions when processing a report the following scenario can happen:
1. You upload an empty report with a flag set to carryforward
2. There will be a new session for that upload (even though it's empty)
3. In the next commit the session will be carried forward

If no real upload was made to clean up old carriedforward sessions from empty uploads
they would accumulate in the report (because `_adjust_sessions` only runs *after* the
processing is done, and *doesn't run* if it's an empty upload). What prompted this
investigation is that a customer was fund with thousands of carryforward session in
some of their commits.

So I'm exploiting the fact that merges into the report need to be made from within a lock
and getting the ID for the session _without_ actually adding it to the original report.

If the report is successful we add it and run `_adjust_sessions`. If the report was empty,
we just never add it to the report, as if it didn't existed.

OBS.: Even though I use the litle exploit I believe that passing the `sessionid` to
`ReportBuilder` is unecessary. We can likely not get a sessionid at all when processin the report
and it would still work. I decided to leave this for it's own change tho.

Also I decided to bump the severity of the messages that indicate a report is empty.
Users are not expected to upload empty reports, so it's at least sensible it should be
a warning.
    @giovanni-guidini
    giovanni-guidini committed Mar 18, 2024
    Configuration menu
    Copy the full SHA
    22be527 View commit details
    Browse the repository at this point in the history

  2. Merge branch 'main' into gio/dont-eager-merge-session

    @giovanni-guidini
    giovanni-guidini authored Mar 18, 2024
    Configuration menu
    Copy the full SHA
    7bba05f View commit details
    Browse the repository at this point in the history