Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Add network connected constraint to DataDonationAnalyticsWorker (EXPOSUREAPP-5286) #2431

Merged

Conversation

kolyaopahle
Copy link
Contributor

Description

This PR adds a network connected constraint to the work builder of the DataDonationAnalyticsWorker as we cannot proceed with analytics donation without a network connection there is no need to enqueue work in that case

Steps to reproduce

Signed-off-by: Kolya Opahle <k.opahle@sap.com>
@kolyaopahle kolyaopahle added backend Issues related to internal work not directly correlated to UI interaction maintainers Tag pull requests created by maintainers prio PRs to review first. labels Feb 22, 2021
@kolyaopahle kolyaopahle added this to the 1.13.0 milestone Feb 22, 2021
@kolyaopahle kolyaopahle requested a review from a team February 22, 2021 13:41
@@ -25,5 +27,11 @@ class DataDonationAnalyticsWorkBuilder @Inject constructor() {
BackgroundConstants.BACKOFF_INITIAL_DELAY,
TimeUnit.MINUTES
)
.setConstraints(buildConstraints())
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to add a log if the worker wasn't started due to the network constraint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if we can, the work is scheduled by the WorkManager api and if there is no network connection the worker is simply not spawned, i don't think we get any kind of callback

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could maybe remove the constraint and do the check inside of the worker ourselves but i think this kind of defeats the purpose of the work manager in the first place

Copy link
Contributor

@chiljamgossow chiljamgossow left a comment

Choose a reason for hiding this comment

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

LGTM

@chiljamgossow chiljamgossow self-assigned this Feb 22, 2021
Copy link
Contributor

@mtwalli mtwalli left a comment

Choose a reason for hiding this comment

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

I confirm it is added :)
Screenshot 2021-02-22 at 15 34 49

@mtwalli mtwalli self-assigned this Feb 22, 2021
@sonarcloud
Copy link

sonarcloud bot commented Feb 22, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@harambasicluka harambasicluka merged commit c4588eb into release/1.13.x Feb 22, 2021
@harambasicluka harambasicluka deleted the fix/5286-add-network-constraint-to-worker branch February 22, 2021 16:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backend Issues related to internal work not directly correlated to UI interaction maintainers Tag pull requests created by maintainers prio PRs to review first.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants