Skip to content

Clean up error handling in NewReleaseFetcher #3279

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
Merged
merged 2 commits into from
Jan 4, 2022
Merged

Clean up error handling in NewReleaseFetcher #3279

merged 2 commits into from
Jan 4, 2022

Conversation

lfkellogg
Copy link
Contributor

  • Cleans up top level Task-based error handling, using TaskUtils.runAsyncInTask
  • Isolates Long parsing error handling into one place
  • Improves messaging around getting IAS artifact IDs, APK hashes, package info

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Dec 29, 2021 •
edited
Loading

Coverage Report

Affected SDKs

  • firebase-app-distribution

    SDK overall coverage changed from 70.18% (3477848) to 70.18% (d4fe1038) by -0.00%.

    Filename Base (3477848) Head (d4fe1038) Diff
    LogWrapper.java 73.33% 64.71% -8.63%
    NewReleaseFetcher.java 82.29% 81.61% -0.68%
    ReleaseIdentificationUtils.java 9.43% 15.52% +6.08%
    TaskUtils.java 95.00% 93.75% -1.25%

Test Logs

Notes

HTML coverage reports can be produced locally with ./gradlew <product>:checkCoverage.
Report files are located at <product-build-dir>/reports/jacoco/.

Head commit (d4fe1038) is created by Prow via merging commits: 3477848 c9d7438.

@lfkellogg lfkellogg force-pushed the lk/new-release-fetcher-tasks branch from 3720a1b to 854ff35 Compare December 29, 2021 22:51
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Dec 29, 2021 •
edited
Loading

Binary Size Report

Affected SDKs

  • firebase-app-distribution

    Type Base (3477848) Head (d4fe1038) Diff
    aar 119 kB 120 kB +1.31 kB (+1.1%)
    apk (release) 1.55 MB 1.55 MB +956 B (+0.1%)

Test Logs

Notes

Head commit (d4fe1038) is created by Prow via merging commits: 3477848 c9d7438.

@lfkellogg
Copy link
Contributor Author

Please hold off on reviewing this I need to make some tweaks.

@lfkellogg lfkellogg removed the request for review from manny-jimenez December 29, 2021 23:08
@lfkellogg lfkellogg force-pushed the lk/new-release-fetcher-tasks branch 2 times, most recently from 8db1a2a to c1892cc Compare December 30, 2021 14:37
@lfkellogg lfkellogg force-pushed the lk/new-release-fetcher-tasks branch from c1892cc to 6817ef2 Compare December 30, 2021 15:04
@lfkellogg
Copy link
Contributor Author

/test smoke-tests

@lfkellogg
Copy link
Contributor Author

/retest

rachaprince
rachaprince approved these changes Jan 4, 2022
View reviewed changes
Copy link

@rachaprince rachaprince left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very nice refactor. I especially love the improvements to the TaskUtils and ReleaseIdentificationUtils classes.

Great work

@lfkellogg lfkellogg merged commit 6178b13 into master Jan 4, 2022
@lfkellogg lfkellogg deleted the lk/new-release-fetcher-tasks branch January 4, 2022 19:25
@firebase firebase locked and limited conversation to collaborators Feb 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rachaprince rachaprince rachaprince approved these changes

@manny-jimenez manny-jimenez manny-jimenez approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lfkellogg @google-oss-bot @rachaprince @manny-jimenez