v1.0.0 BREAKING CHANGES release thread

[tombh]

With huge thanks and kudos to Esbonio's @alcarney and Microsoft's @karthiknadig we now have a pre release version of Pygls that migrates away from Pydantic and to lsprotocol.

⚠️ This involves breaking changes. Existing projects depending on Pygls will need to update their code. Testers are welcome.

You can try it by explicitly setting your dependency to pygls=1.0.0a2. The migration guide is at: docs/pages/migrating-to-v1.rst

lsprotocol is mostly just a module that officially and comprehensively defines the data structures and types that make up the LSP specification. Therefore, what it offers are upstream (from Microsoft, the creators of LSP) guarantees on adherence to the formal behaviour and expectations of the specification.

TODO:

• Write Publish guide on how to make the required changes.
• Isolate a commit in a sample LSP server project that demonstrates the changes. Eg; jedi-language-server already has a PR. Or maybe make a verbose commit in the new "Pygls Starter project" I'm working on.
• Get feedback from people that have successfully made the migration:
  • pygls 1.0.0a0 pappasam/jedi-language-server#230.
  • Upgrade to pygls==1.0.0a3 astral-sh/ruff-vscode#37
• @alcarney what else would you like to see happen before the final release?
• Remove advice to start new projects with v1 alpha

This may also be a good opportunity to introduce other breaking changes, such as in #274

Notes:

• Issues that this release should either address or fix:
  • Why Pydantic? #221
  • pydantic throws ValidationError #243
  • Crashes on Python 3.11 #234
  • Create new package for protocol-level concerns #101
  • Support pydantic 1.10 (Probably best to support 1.9+) #275
• See the original "lsprotocol" PR for further discussion.

Edit: added line about taking advantage of a breaking change release

[dimbleby]

My main observation from pappasam/jedi-language-server#230 is that it's a real shame that lsprotocol doesn't expose types, that will make authoring language servers that little bit more error prone than it need be.

Otherwise it all seems to have gone pretty smoothly, nice job.

[karthiknadig]

@dimbleby if you see anything missing, please file bug on lsprotocol. I can try and improve code generation to produce any missing or supporting types.

[dimbleby]

@karthiknadig you probably want to get involved in the bug at pappasam/jedi-language-server#230 (comment)

[tombh]

@dimbleby Wow that's awesome that you've already managed to integrate this. Can you explain more about lsprotocol not exposing types? I mean I see that it does use Python types, eg: types.py. So do you mean that it lacks a py.typed stub to help tools like Mypy orient themselves? Or do you mean that it doesn't give type-based feedback during serialisation/deserialisation? Or something else?

[dimbleby]

I mean type annotations, I would love for lsprotocol to (i) be clean under mypy --strict and (ii) include a py.typed so that users could take advantage of that

you'll likely have seen that it's not quite so awesome as all that, pappasam/jedi-language-server#230 (comment) looks like an lsprotocol bug

[tombh]

Ok, so it's more a matter of having better typing support, rather than introducing types in the first place? It seems like lsprotocol very much leans in the direction of wanting to fully support Mypy right? So it should be a legitimate next step to explore, or even that @karthiknadig intends for full support and we've just surfaced some minor bugs?

[dimbleby]

some way still to go, if mypy support is intended:

$ mypy lsprotocol
...
Found 25 errors in 2 files (checked 5 source files)

$ mypy --strict lsprotocol
...
Found 68 errors in 4 files (checked 5 source files)

Actually that's a relatively small number for a project that perhaps hasn't yet even tried - possibly it wouldn't be tremendously hard to get to zero errors.

[karthiknadig]

Created microsoft/lsprotocol#96 to track mypy --strict issues.

[alcarney]

Not sure how we want to handle issues with this branch? Do we want to create dedicated issues but tag them with something like v1, or just track everything in this PR?

I've noticed that the server process just completely crashes if it encounters a message it cannot parse/does not like.
Happy to open a dedicated issue for this if we want to go that route :)

[tombh]

Good question @alcarney, it probably depends on the issue. I think that one may need its own issue, because it's more likely to be a bug in Pygls itself right?

[tombh]

Just a little update to summarise from the other threads involved.

• @karthiknadig released a new version of lsprotocol that fixes an issue that was identified in the Jedi server integration pygls 1.0.0a0 pappasam/jedi-language-server#230 (comment)
• @dimbleby experimented with adding a py.typed marker file to their local copy of lsprotocol and successfully got typing information. This is good news for Address issues reported by mypy --strict microsoft/lsprotocol#96

[alcarney]

@tombh yes, the error handling in pygls needs some refinement as one bad message probably shouldn't take out the server completely

[tombh]

I've made a new issue for the more general problem of unhandled errors: #277

[tombh]

I've just rebased the recent changes we've merged into master in the last few days, most notably improved error handling #282. That addresses an underlying issue that @alcarney noticed with this new code, namely that the server wasn't catching unhandled deserialisation errors, which I think understandably could come up more now that we've changed all the underlying types.

But I think @alcarney, you we still need to fix that error? All we've done with #282 is catch unhandled errors, rather than fix the unhandled error right?

Also, where are we at with the Mypy types? I don't think that's necessarily a blocking issue? The main thing is that https://pypi.org/project/lsprotocol/2022.0.0a6 fixed the errors in pappasam/jedi-language-server#230 (comment) right?

[alcarney]

But I think @alcarney, we still need to fix that error?

The error in question was being tracked in microsoft/lsprotocol#102, and should be fixed in the next version of lsprotocol

[tombh]

Ah great! I'll look out for that then

[tombh]

📰 New versions news 📰

• https://github.com/microsoft/lsprotocol/releases/tag/2022.0.0a7 Contains some fixes applicable to this branch
• https://pypi.org/project/pygls/1.0.0a2 Just tracking the master branch

So it looks like we're getting there with this branch, or indeed are there already. I think the only main thing left to do is write a brief v0.12.x to v1 migration guide.

[alcarney]

Hmm... I wonder why all the builds have hung...

I think the only main thing left to do is write a brief v0.12.x to v1 migration guide.

I can put something together as I have a go at porting esbonio - unless anyone else has started working on one?

So it looks like we're getting there with this branch

Exciting! Though do we know how many servers have attempted a migration?

[tombh]

No, nobody else (well it would only be me anyway) has started on a migration guide. I think you'd be best placed to write it, so if you're up for that then great, but I'm totally happy to do it too.

The only report from the wild about implementing this migration so far that we've had is pappasam/jedi-language-server#230 Yeah, would be nice to get some more feedback for sure.

So, the CI errors. There are 3 interesting things happening.

1. The LSP client that we use in tests, which is repurposed from the LSP server, doesn't (really?? still!?) have a central place to timeout asyncio futures 🫤 Hence the hung CI. Point 3 here in particular is the culprit for the hang. Let me think about a way to refactor the LSP test client so that at the very least CI doesn't hang again like this.

   PS. I assume that the fact that it's the unique circumstances of the repurposed LSP test client, that this hanging behaviour doesn't reflect a potential issue with the actual server itself. I'm not 100% sure though. I just assume that the server doesn't call anything like a timeout-less, blocking Future.result() in non-async functions 👀

2. CodeActionKind.Refactor isn't parsing:

E       AssertionError: assert 'refactor' == <CodeActionKind.Refactor: 'refactor'>
E        +  where 'refactor' = CodeAction(title='action2', kind='refactor', diagnostics=None, is_preferred=None, disabled=None, edit=None, command=None, data=None).kind
E        +  and   <CodeActionKind.Refactor: 'refactor'> = CodeActionKind.Refactor

3. FoldingRange isn't parsing:
   Can be isolated with: .tox/py/bin/python3.10 -m pytest -vvv tests/lsp/test_folding_range.py::test_folding_range_return_list --log-cli-level=DEBUG

| Traceback (most recent call last):
|   File "<cattrs generated structure lsprotocol.types.FoldingRange>", line 28, in structure_FoldingRange
|     res['kind'] = __c_structure_kind(o['kind'], __c_type_kind)
|   File "/home/streamer/Workspace/pygls/.tox/py/lib/python3.10/site-packages/cattrs/converters.py", line 377, in _structure_error
|     raise StructureHandlerNotFoundError(msg, type_=cl)
| cattrs.errors.StructureHandlerNotFoundError: Unsupported type: typing.Union[lsprotocol.types.FoldingRangeKind, str, NoneType]. Register a structure hook for it.
| Structuring class FoldingRange @ attribute kind

[tombh]

@charliermarsh Yes, that is indeed helpful to see! Thank you ❤️

Everybody: I've updated Pygls' README and docs to state that v1 alpha is now the recommended version to start new Pygls-based LSP servers.

Having released a fix on our main branch for a LSP type issue that is already fixed in this branch, I'm feeling like we're ready to release this branch.

Today, the fact that this branch is not released caused a tiny bit more real extra work rather than the theoretical extra work that might come from releasing this branch. Which is to say, there's a reasonable assumption that this branch will not cause any extra work when released, and an actual fact that not releasing it has already created a tiny bit more work.

Fixing the LSP type issue today felt very much like supporting an old version of Pygls. Which is something we can still totally do even once this v1 branch is released.

What do you think @alcarney?

[alcarney]

I agree, I can't think of a technical reason to hold this branch back anymore.

The only other thing that's probably worth doing before a release, is to make sure the docs, README etc are consistent with all the changes here.

[tombh]

Great. I've just pushed those doc changes and bumped the version to v1.0.0. I'm ready to merge! Sanity-check depending of course.

[alcarney]

Looks good.

One final thought, should we wait for a non-alpha release of lsprotocol? Technically, I don't think it matters, but I'm (possibly overthinking!) how it would look to someone unfamiliar with the details when they see a 1.0 pulling in an alpha library?

[brettcannon]

One final thought, should we wait for a non-alpha release of lsprotocol?

FYI Karthik won't be available to make a final release until the latter half of this month, but we would be happy to if the library is working as you all need it to be.

[tombh]

@alcarney Good point. Ideally yes, it'd be good to release with a non-alpha lsprotocol. I wonder if we're in a bit of a chicken and the egg situation? That's the main reason I'm voting for a release now, just to fall on one side of the line. It's only a vote, so totally happy to wait as well. But I'm feeling like this v1 branch now offers something better than current v0.x. Even if it might not be perfect.

@brettcannon From my perspective I think pygls and lsprotocol are ready and am happy to even release pygls with the alpha lsprotocol. So take that as one vote of confidence, but of course let the general consensus be the guide.

[alcarney]

@tombh, just remembered another change I think we wanted to make so I've opened #295.

Happy to release v1.0 with an alpha lsprotocol if you are, just thought I'd check 🙂

[tombh]

Well, LGTM then 🚀

[alcarney]

Can't believe it's actually happening! 😅

[tombh]

Woohoo! 🎉 Merged and published. Thank you everybody ❤️🙇

I actually streamed the moment live on Twitch 😎 https://clips.twitch.tv/DeadExquisiteButterPJSugar-DLXXPnknw87VSdPs

Here's the Pypi package: https://pypi.org/project/pygls/1.0.0

And the migration docs have been published too: https://pygls.readthedocs.io/en/latest/pages/migrating-to-v1.html (I notice I didn't get the list syntax right for the links to other already migrated projects, I'll make a little PR for that).

[charliermarsh]

Congratulations on the release!

[brettcannon]

Congrats on the release! And thanks to everyone for providing feedback on lsprotocol so we could improve it for everyone's benefit!