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

PR: Fix enabling/disabling individual providers (Completions) #21463

Merged
merged 1 commit into from
Oct 28, 2023

Conversation

dalthviz
Copy link
Member

Description of Changes

The start and shutdown providers logic was missing updating the language_status attribute which is used to determine the providers that are available for a certain language. In the case of a completion request all responses for the available providers need to be aggregated, so all providers need to be waited before a response is made available. This caused that by disabling one of the providers no reponse was able to be retrieved but still that provider was expected to give a reponse

Also, there is some logic to handle a timeout but only the LSP provider is marked as slow and at least 3 providers need to be marked as slow to trigger the logic. For more details about that see:

# Start the timer on this request
if req_type in self.AGGREGATE_RESPONSES and slow_provider_count > 2:
if self.wait_for_ms > 0:
QTimer.singleShot(self.wait_for_ms,
lambda: self.receive_timeout(req_id))
else:
self.requests[req_id]['timed_out'] = True

Issue(s) Resolved

Fixes #21462

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: dalthviz

@ccordoba12
Copy link
Member

Thanks @dalthviz for quickly finding the solution to this serious problem. For now I'm just going to leave a quick note to say that the failures in our tests on Windows are unrelated to this and I'll try to fix them tomorrow.

@ccordoba12
Copy link
Member

@dalthviz, please rebase on top of 5.x to get the changes I did in PR #21467 to fix our tests.

Also, there is some logic to handle a timeout but only the LSP provider is marked as slow and at least 3 providers need to be marked as slow to trigger the logic.

I suggest to leave that for Spyder 6 so we can think more carefully about it. What do you think?

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @dalthviz!

@ccordoba12 ccordoba12 merged commit c34ee74 into spyder-ide:5.x Oct 28, 2023
22 checks passed
ccordoba12 added a commit that referenced this pull request Oct 28, 2023
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.

None yet

2 participants