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

Report progress even when initialization fails #381

Merged
merged 2 commits into from
Jun 29, 2023

Conversation

syphar
Copy link
Contributor

@syphar syphar commented May 16, 2023

works around the problem in #374.

We will now start still reporting progress even when the token initialization is timing out, which happens mostly happens in synchronous handlers.

I also added an option to explicitly skip the token initialization for handlers that already know they are synchronous, so we don't have to wait for the timeout.

In a new feature, or if we would accept breaking changes, we IMO actually shouldn't catch the timeout here. So any synchronous handlers explicitly have to buy into skipping token initialization. But this would be breaking for each plugin that uses progress

@syphar
Copy link
Contributor Author

syphar commented May 16, 2023

cc @ccordoba12 what do you think about this?

@syphar syphar force-pushed the optional-progress-init branch 2 times, most recently from 6ca537a to 62bb26b Compare May 16, 2023 18:19
@ccordoba12 ccordoba12 changed the title report progress even when initialization fails Report progress even when initialization fails May 20, 2023
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.

Thanks @syphar for your continued help with this! I left a couple of minor suggestions, otherwise looks good to me.

pylsp/workspace.py Outdated Show resolved Hide resolved
pylsp/workspace.py Outdated Show resolved Hide resolved
@ccordoba12
Copy link
Member

In a new feature, or if we would accept breaking changes, we IMO actually shouldn't catch the timeout here. So any synchronous handlers explicitly have to buy into skipping token initialization. But this would be breaking for each plugin that uses progress

Do you know what external plugins are using report progress?

@ccordoba12 ccordoba12 added this to the v1.7.4 milestone May 20, 2023
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
@syphar
Copy link
Contributor Author

syphar commented May 21, 2023

In a new feature, or if we would accept breaking changes, we IMO actually shouldn't catch the timeout here. So any synchronous handlers explicitly have to buy into skipping token initialization. But this would be breaking for each plugin that uses progress

Do you know what external plugins are using report progress?

No plugin I use, but I don't use many.

Btw, with this change we could re-add the progress pieces we removed in the last version.

@ccordoba12
Copy link
Member

Btw, with this change we could re-add the progress pieces we removed in the last version.

Ok, please do that in a follow up PR.

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.

Thanks @syphar!

@ccordoba12 ccordoba12 merged commit fd4a0df into python-lsp:develop Jun 29, 2023
10 checks passed
@syphar syphar deleted the optional-progress-init branch June 29, 2023 18:43
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jul 5, 2023
## Version 1.7.4 (2023/06/29)

### Issues Closed

* [Issue 393](python-lsp/python-lsp-server#393) - Environment path doesn't expand user directory

In this release 1 issue was closed.

### Pull Requests Merged

* [PR 394](python-lsp/python-lsp-server#394) - Resolve homedir references in Jedi environment path, by [@odiroot](https://github.com/odiroot)
* [PR 381](python-lsp/python-lsp-server#381) - Report progress even when initialization fails, by [@syphar](https://github.com/syphar)
* [PR 380](python-lsp/python-lsp-server#380) - Fix pylint hang on file with many errors, by [@hetmankp](https://github.com/hetmankp)

In this release 3 pull requests were closed.
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.

None yet

2 participants