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

Revert "feat(android) Check server is reachable before starting background backup (#8989)" #9324

Merged
merged 2 commits into from
May 9, 2024

Conversation

alextran1502
Copy link
Contributor

This reverts commit 0435de5.

Copy link

cloudflare-pages bot commented May 8, 2024

Deploying immich with  Cloudflare Pages  Cloudflare Pages

Latest commit: e3add5b
Status: ✅  Deploy successful!
Preview URL: https://3b149f87.immich.pages.dev
Branch Preview URL: https://test-sync-issue-apk.immich.pages.dev

View logs

@alextran1502
Copy link
Contributor Author

@devjn Hey man, I just want to give you a poke about the potential revert of your PR; it seems to be a cause of issue #9295, which has been reported since the latest release. Do you have any thoughts/ideas on what could be the cause?

@alextran1502 alextran1502 marked this pull request as ready for review May 8, 2024 17:46
@devjn
Copy link
Contributor

devjn commented May 8, 2024

@devjn Hey man, I just want to give you a poke about the potential revert of your PR; it seems to be a cause of issue #9295, which has been reported since the latest release. Do you have any thoughts/ideas on what could be the cause?

Hi,
Honestly it should have no effect at all for iOS and shouldn't affect non background backup.

One idea I have is that probably connection is not aware of proxy.
Can try to change it with this:

val proxy = Proxy(Proxy.Type.HTTP, InetSocketAddress("auto", 0)) // Use "auto" to detect proxy
val urlConnection = URL(url).openConnection(proxy) as HttpURLConnection

Or if it is reverse proxy refusing, might be request method should be changed to "GET". From what I read it seems some reverse proxies deny "HEAD" requests

@alextran1502
Copy link
Contributor Author

@devjn Since we don't have a proper way to test this ourselves, I think I will revert this and reopen the PR with your suggestion of adding the Proxy check and then test it for the next release. Does that sound good?

@devjn
Copy link
Contributor

devjn commented May 9, 2024

@devjn Since we don't have a proper way to test this ourselves, I think I will revert this and reopen the PR with your suggestion of adding the Proxy check and then test it for the next release. Does that sound good?

Yeah, sounds alright.

@alextran1502 alextran1502 linked an issue May 9, 2024 that may be closed by this pull request
3 tasks
@alextran1502 alextran1502 merged commit 55031cc into main May 9, 2024
23 of 24 checks passed
@alextran1502 alextran1502 deleted the test-sync-issue-apk branch May 9, 2024 17:16
@sidamos sidamos mentioned this pull request May 14, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mobile app fails to show new assets
2 participants