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

Use of command palette cancels previously-running non-thread workers #3615

Closed
davep opened this issue Oct 30, 2023 Discussed in #3613 · 6 comments · Fixed by #3618
Closed

Use of command palette cancels previously-running non-thread workers #3615

davep opened this issue Oct 30, 2023 Discussed in #3613 · 6 comments · Fixed by #3618
Assignees
Labels
bug Something isn't working

Comments

@davep
Copy link
Contributor

davep commented Oct 30, 2023

Discussed in #3613

Originally posted by otherjason October 30, 2023
I have an app that uses several long-lived workers to asynchronously poll external services for data and update the UI. I found that closing the command palette, either via ESC or by selecting a command, seems to cancel any pending workers. Here's a small example:

import asyncio
from textual import work
from textual.app import App, ComposeResult
from textual.widgets import RichLog

class Test(App):
    CSS = '''
    RichLog {
        height: 100%;
    }
    '''
    def compose(self) -> ComposeResult:
        yield RichLog(id = 'log')

        self.my_worker()

    @work(exclusive = False)
    async def my_worker(self):
        counter = 1

        log = self.query_one('#log')
        while True:
            log.write(f'test {counter}')
            counter += 1
            await asyncio.sleep(1)

if __name__ == "__main__":
    Test().run()

When running the above, it will print a new line to the RichLog every second. If you open the command palette, then the worker stays running. But it seems that after the palette comes up, as soon as I press a key, the worker stops running.

Is this behavior intended, or is this a bug?

@davep davep added the bug Something isn't working label Oct 30, 2023
@davep
Copy link
Contributor Author

davep commented Oct 30, 2023

Hat tip to @otherjason for reporting this.

Bumping straight to the todo list as it's a bit of a show-stopper.

@davep
Copy link
Contributor Author

davep commented Oct 30, 2023

As mentioned in the discussion, I would suspect that:

self.workers.cancel_all()

is doing more than I expected it to (I expected self.workers to be all the workers running against the current screen/widget). Don't know for sure, but that's the first thing I'd double-check.

Also, in retrospect, it might make sense to ensure that any worker in the command palette is in an "internal" group of some sort?

@rodrigogiraoserrao
Copy link
Contributor

rodrigogiraoserrao commented Oct 30, 2023

self.workers is a property that points to self.app.workers, so the fact that self.workers.cancel_all() is stopping all workers definitely looks like it is intended behaviour.

Having self.workers be an alias to self.app.workers looks like a terrible idea to me...

And I guess you're right in that it makes sense to have the command palette workers have an internal group of some sort.

@rodrigogiraoserrao
Copy link
Contributor

rodrigogiraoserrao commented Oct 30, 2023

You'd probably want to use either self.workers.cancel_node(self), or self.workers.cancel_group with the aforementioned worker group that needs to be created.

@davep davep self-assigned this Oct 31, 2023
@davep
Copy link
Contributor Author

davep commented Oct 31, 2023

Having self.workers be an alias to self.app.workers looks like a terrible idea to me...

Yeah, that's a very curious choice. I wonder if we should review that?

@github-actions
Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants