Skip to content

fix(GeneratorQueryHandler): replay fetchMore requests made while active#1618

Open
brenns10 wants to merge 1 commit into
albertlauncher:mainfrom
brenns10:fix_stalled_generator
Open

fix(GeneratorQueryHandler): replay fetchMore requests made while active#1618
brenns10 wants to merge 1 commit into
albertlauncher:mainfrom
brenns10:fix_stalled_generator

Conversation

@brenns10

Copy link
Copy Markdown

Hello, I ended up digging in a bit and finding a fix (and test case) for the issue I reported earlier with the Python plugin. With this change my example plugin works as expected.

To be totally transparent - I did use AI/LLM tools in this PR, but I have read and understood everything I'm submitting here.

Do not drop fetchMore calls that arrive while a batch is still active.
Queue one pending fetch request and replay it after activeChanged(false).

This handles reentrant fetchMore calls triggered during results insertion
and prevents multi-chunk generators from stalling after the first chunk.

Fixes: albertlauncher/albert-plugin-python#12
@ManuelSchneid3r

Copy link
Copy Markdown
Member

digging in a bit and finding a fix

ty, once again steph.

AI/LLM tools in this PR,

thats okay unless i am the first to review their output ;D

Regarding this PR: In my opinion, this is really hard to argue without a formal protocol for the lazy fetch mechanism using canFetchMore() / fetchMore(). I could not find any yet. There is an async example, though, which suggests that the protocol should work with async data sources like QueryExecution.

It makes sense to assume that the view calls canFetchMore() and fetchMore() as soon as the model reports new items and the view is still not filled. However, without a formal protocol, I can only speculate. This is probably going to take some more time to fix. In the absence of a protocol, I will have to dig into the source code of the views. If it behaves as described above, the issue should not exist, I guess.

@ManuelSchneid3r

Copy link
Copy Markdown
Member

Given that you sent a fix already I have to note that the entire query handling related code is really easy to mess up and I have been bitten by nasty bugs stemming from Qt itself. Although this PR may fix the build for your Qt version this may not hold for all others out there. I just want to really understand what is going on there before solving a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants