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

Improve async compatibility of app store #177

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MatthewWilkes
Copy link
Member

This moves some code from the background task to the main task, and refactors it to allow use of async methods. This looks like these tasks were in background from the idea that the background task runs 'in the background', rather than runs 'when the app is in the background'.

There are two calls that use requests.get, which block the update task. By refactoring slightly to use async, we can use async_helpers.unblock to move them to a thread.

This moves some code from the background task to the main task, and refactors it to allow use of async methods. This looks like these tasks were in background from the idea that the background task runs 'in the background', rather than runs 'when the app is in the background'.

There are two calls that use requests.get, which block the update task. By refactoring slightly to use async, we can use async_helpers.unblock to move them to a thread.
Copy link
Contributor

@npentrel npentrel left a comment

Choose a reason for hiding this comment

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

I can't test this right now but I think there's one issue?

get, render_update, APP_STORE_LISTING_URL
)
except Exception:
self.update_state("no_index")
Copy link
Contributor

Choose a reason for hiding this comment

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

This would get changed in the next line wouldn't it

Comment on lines +259 to +265
try:
self.response = await async_helpers.unblock(
get, render_update, APP_STORE_LISTING_URL
)
except Exception:
self.update_state("no_index")
self.update_state("index_received")
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, per Naomi's comment I think this ought to be like this?

Suggested change
try:
self.response = await async_helpers.unblock(
get, render_update, APP_STORE_LISTING_URL
)
except Exception:
self.update_state("no_index")
self.update_state("index_received")
try:
self.response = await async_helpers.unblock(
get, render_update, APP_STORE_LISTING_URL
)
self.update_state("index_received")
except Exception:
self.update_state("no_index")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants