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

feat(android) Check server is reachable before starting background backup #8594

Merged

Conversation

devjn
Copy link
Contributor

@devjn devjn commented Apr 7, 2024

Add a check for android version that verifies that server is reachable before starting background backup and fail otherwise.

Why?

When server is behind VPN there is no reliable way to set constrains that would allow to start only for specific network.
This results in backup work tries that hang up in notifications.
One way to tackle this would be to start backup only when specific network is connected. Unfortunately it seems to be impossible when using VPN.
Another option is to check that server url can actually be reached before trying to run backup.

This PR tries to accomplish this by opening HttpURLConnection and checking for 200 code.

I am open for any better solutions if any.

@alextran1502
Copy link
Contributor

Hello, thank you for the PR.

Have you performed any tests on a physical device with this change and confirm that it works as expected?

@devjn
Copy link
Contributor Author

devjn commented Apr 8, 2024

Hello, thank you for the PR.

Have you performed any tests on a physical device with this change and confirm that it works as expected?

Hi,
Yes I have tested it on physical android 14 device and confirmed that it works as expected.

@alextran1502 alextran1502 self-assigned this Apr 15, 2024
Copy link
Contributor

@fyfrey fyfrey left a comment

Choose a reason for hiding this comment

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

tested in an emulator. background backup still works fine.

code looks reasonable

@alextran1502 alextran1502 merged commit 71b6d8b into immich-app:main Apr 20, 2024
23 checks passed
alextran1502 added a commit that referenced this pull request Apr 20, 2024
alextran1502 added a commit that referenced this pull request Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants