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

Adding delay function #1431

Closed
wants to merge 4 commits into from
Closed

Adding delay function #1431

wants to merge 4 commits into from

Conversation

VahilaK
Copy link

@VahilaK VahilaK commented Feb 21, 2022

Add a function which facilitates to run a task after some time in background.

This PR fixes #1415

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@codecov
Copy link

codecov bot commented Mar 6, 2022

Codecov Report

Merging #1431 (bd55e0e) into master (01df836) will decrease coverage by 0.09%.
The diff coverage is 40.00%.

Impacted Files Coverage Δ
src/core/toga/app.py 84.80% <40.00%> (-1.13%) ⬇️

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR; while this does implement functionality that will run on a delay, it's not exactly what #1415 was requesting.

The original request was to install a background task, and that task included a 5 second delay.

This PR implements an asynchronous method that does a sleep as it's first step.

The reason this matters is that you can only invoke your version of the method from an asynchronous context. This means it can't be invoked from a standard button handler, or from a generator.

@freakboy3742 freakboy3742 added the not quite right The idea or PR has been reviewed, but more work is needed. label Apr 6, 2022
@VahilaK
Copy link
Author

VahilaK commented Aug 20, 2022

@freakboy3742 I apologize for the delay in reverting. I understand there would be an issue with asynchronous context but I am not really sure on how to take this forward. One idea I have is to create a new thread which would add the method to background after the thread sleeps for the timeout duration. I am planning to use time.sleep in the new thread, this way there wouldn't be any asynchronous functions. Please let me know your views on this approach.

@freakboy3742
Copy link
Member

@freakboy3742 I apologize for the delay in reverting. I understand there would be an issue with asynchronous context but I am not really sure on how to take this forward. One idea I have is to create a new thread which would add the method to background after the thread sleeps for the timeout duration. I am planning to use time.sleep in the new thread, this way there wouldn't be any asynchronous functions. Please let me know your views on this approach.

There's no need to use threads - this can be done entirely with asyncio. However, you need to pay close attention to what it is that you're putting on the background task list.

Your implementation here is "sleep for 2 seconds and then call a function". What we need is "call a function right now whose first action is to sleep for 2 seconds".

@freakboy3742
Copy link
Member

Closing due to lack of activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not quite right The idea or PR has been reviewed, but more work is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delay or after function
2 participants