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

(Initial) support for pep0484 annotation type hints #661

Merged
merged 21 commits into from Dec 20, 2015

Conversation

Projects
None yet
5 participants
@reinhrst
Contributor

reinhrst commented Dec 14, 2015

Per request PR to linter branch for #170 .

Handled the comments on #659 , new/more comments are always welcome!

It should now recognise simple function parameter and return type annotations (this means: single built in or custom classes, including forward referencing).

reinhrst added some commits Dec 13, 2015

Build in version-dependency in integration tests
If a line is encountered with the comment  or , then the tests are skipped if the current python version is less than the requested one. All tests until the end of the file, or a new comment specifying a compatibe python version are skipped
clean out the last_* fields of sys before importing it.
The system gets confused if there were uncaught errors in previous
tests without this. Particularly, it crashes (at least 2.6) if any tests during
test_integrations were skipped.
@reinhrst

This comment has been minimized.

Show comment
Hide comment
@reinhrst

reinhrst Dec 14, 2015

Contributor

Coverage became less since coverage seems to run as python 2, and my changes are python3 mostly

Contributor

reinhrst commented Dec 14, 2015

Coverage became less since coverage seems to run as python 2, and my changes are python3 mostly

@reinhrst reinhrst changed the title from Linter to (Initial) support for pep0484 annotation type hints Dec 14, 2015

@davidhalter

View changes

Show outdated Hide outdated jedi/parser/tree.py
@davidhalter

View changes

Show outdated Hide outdated test/test_api/test_api.py
@davidhalter

This comment has been minimized.

Show comment
Hide comment
@davidhalter

davidhalter Dec 14, 2015

Owner

This is very awesome. One of the best pull requests out here, ever :) I will merge as soon as the last remaining issues are clear.

However running the 2.6 tests without this fails.

Well, for the moment being I guess I would just accept failing 2.6 tests. I can then later debug it. I don't think this is the right choice now.

Owner

davidhalter commented Dec 14, 2015

This is very awesome. One of the best pull requests out here, ever :) I will merge as soon as the last remaining issues are clear.

However running the 2.6 tests without this fails.

Well, for the moment being I guess I would just accept failing 2.6 tests. I can then later debug it. I don't think this is the right choice now.

Revert "clean out the last_* fields of sys before importing it."
This reverts commit be399c8.
Will break python 2.6 (possibly 2.7) tests; this is expected behaviour.
See #661 (comment)
@reinhrst

This comment has been minimized.

Show comment
Hide comment
@reinhrst

reinhrst Dec 14, 2015

Contributor

Cool, thanks! Reverted the test-fix; was just happy to get the tests green last night, but indeed I think it's better to actually dive into the problem and understand it.

Was running this branch/PR in my dev environment today, very happy so far :)

Contributor

reinhrst commented Dec 14, 2015

Cool, thanks! Reverted the test-fix; was just happy to get the tests green last night, but indeed I think it's better to actually dive into the problem and understand it.

Was running this branch/PR in my dev environment today, very happy so far :)

@davidhalter

View changes

Show outdated Hide outdated jedi/parser/tree.py
@reinhrst

This comment has been minimized.

Show comment
Hide comment
@reinhrst

reinhrst Dec 15, 2015

Contributor

Also I'd like to ask your opinion on something.

PEP-0484 leads to lots of circular references, since the modules that define the classes need to be imported in module-scope in the modules that have functions either accepting or returning those references, regardless of whether one uses forward references or not.

As far as I have been able to test, there is no nice way to do circular references in python 3.4 in packages (see also my question here). Even the example code in the Forward Reference section of the PEP doesn't work on 3.4. It does work on 3.5, however I haven't been willing to really dig into what changed.

I noticed that docstring imports allow specifying a type, where the import is being done implicit (i.e. one can say :type x: model.a.A, even if model isn't imported on module level). Would it make sense to consider something like this for pep0484-like annotations (note that this makes them not correct pep0484 annotations anymore)? Or does it make more sense to ask people to move to python 3.5 (provided that it gets jedi support at some point in the future)?

Contributor

reinhrst commented Dec 15, 2015

Also I'd like to ask your opinion on something.

PEP-0484 leads to lots of circular references, since the modules that define the classes need to be imported in module-scope in the modules that have functions either accepting or returning those references, regardless of whether one uses forward references or not.

As far as I have been able to test, there is no nice way to do circular references in python 3.4 in packages (see also my question here). Even the example code in the Forward Reference section of the PEP doesn't work on 3.4. It does work on 3.5, however I haven't been willing to really dig into what changed.

I noticed that docstring imports allow specifying a type, where the import is being done implicit (i.e. one can say :type x: model.a.A, even if model isn't imported on module level). Would it make sense to consider something like this for pep0484-like annotations (note that this makes them not correct pep0484 annotations anymore)? Or does it make more sense to ask people to move to python 3.5 (provided that it gets jedi support at some point in the future)?

@davidhalter

This comment has been minimized.

Show comment
Hide comment
@davidhalter

davidhalter Dec 16, 2015

Owner

there is no nice way to do circular references in python 3.4 in packages

This might actually be connected to https://bugs.python.org/issue992389. I don't really get what the current status of that issue is, but I'm wondering that the forward reference code is even working in Python 3.5.

# File models/a.py
from models import b
print('A')

# File models/b.py
from models import a
print('B')

This code has failed in pretty much every version of Python I know. If it doesn't actually fail anymore I will hug the person that did it.

Ok. Now that I've tested it, it does actually finally work in Python 3.5. I'm so happy (and baffled)! :) Please whoever you are who fixed this. I want to hug you :-) You have just fixed my biggest annoyance in Python!!!

Awesome!

IMO breaking the PEP doesn't make sense, because it actually works for Python 3.5. Python 3.4 is not far away from Python3.5 compatibility-wise. Because it all works well in Python 3.5.

BTW: The new import logic is a little bit strange: These "circular modules" are executed twice. This means when printing you will get:

$ python3.5 -m circular.b
B
A
B

READY TO MERGE? :)

Owner

davidhalter commented Dec 16, 2015

there is no nice way to do circular references in python 3.4 in packages

This might actually be connected to https://bugs.python.org/issue992389. I don't really get what the current status of that issue is, but I'm wondering that the forward reference code is even working in Python 3.5.

# File models/a.py
from models import b
print('A')

# File models/b.py
from models import a
print('B')

This code has failed in pretty much every version of Python I know. If it doesn't actually fail anymore I will hug the person that did it.

Ok. Now that I've tested it, it does actually finally work in Python 3.5. I'm so happy (and baffled)! :) Please whoever you are who fixed this. I want to hug you :-) You have just fixed my biggest annoyance in Python!!!

Awesome!

IMO breaking the PEP doesn't make sense, because it actually works for Python 3.5. Python 3.4 is not far away from Python3.5 compatibility-wise. Because it all works well in Python 3.5.

BTW: The new import logic is a little bit strange: These "circular modules" are executed twice. This means when printing you will get:

$ python3.5 -m circular.b
B
A
B

READY TO MERGE? :)

@reinhrst

This comment has been minimized.

Show comment
Hide comment
@reinhrst

reinhrst Dec 17, 2015

Contributor

Yuk, I certainly have some code in modules that will break if it's executed twice, I think....

The more I think about circular references, the more I think it will get worse before it will get better. I recon that to PEP-0484 annotate everything correctly, you will basically end up with everything circular-importing each other, which means that any idea of a dependable import order totally disappears. And all annotations everywhere should be made as forward references. But I expect this is something that a next version of pep-0484 will have to look at.

Yep ready to merge, let me know what to do with the assert, and I think we're on our way :)

Contributor

reinhrst commented Dec 17, 2015

Yuk, I certainly have some code in modules that will break if it's executed twice, I think....

The more I think about circular references, the more I think it will get worse before it will get better. I recon that to PEP-0484 annotate everything correctly, you will basically end up with everything circular-importing each other, which means that any idea of a dependable import order totally disappears. And all annotations everywhere should be made as forward references. But I expect this is something that a next version of pep-0484 will have to look at.

Yep ready to merge, let me know what to do with the assert, and I think we're on our way :)

@davidhalter

This comment has been minimized.

Show comment
Hide comment
@davidhalter

davidhalter Dec 17, 2015

Owner

I pulled the changes and I will locally make a few modifications (mostly stuff that you haven't implemented yet) and I will then push it back.

If you have any ideas about implementing the typing.py library, I'm happy to listen!

Owner

davidhalter commented Dec 17, 2015

I pulled the changes and I will locally make a few modifications (mostly stuff that you haven't implemented yet) and I will then push it back.

If you have any ideas about implementing the typing.py library, I'm happy to listen!

@davidhalter davidhalter merged commit 160b6fc into davidhalter:linter Dec 20, 2015

@davidhalter

This comment has been minimized.

Show comment
Hide comment
@davidhalter

davidhalter Dec 20, 2015

Owner

I merged. I think most issues are solved. Anything else can be done at a later stage as well! (I have locally edited the parser a bit to allow for a better control in what's ok in forward references.)

Shall we tackle the typing module, now? :)

Thanks again so much for your help! Really appreciate it, it was a pleasure working with you!

Owner

davidhalter commented Dec 20, 2015

I merged. I think most issues are solved. Anything else can be done at a later stage as well! (I have locally edited the parser a bit to allow for a better control in what's ok in forward references.)

Shall we tackle the typing module, now? :)

Thanks again so much for your help! Really appreciate it, it was a pleasure working with you!

@reinhrst

This comment has been minimized.

Show comment
Hide comment
@reinhrst

reinhrst Dec 21, 2015

Contributor

Thanks! It was certainly a nice experience to have a PR made and merged within a week :D. I can very much imagine I will want to contribute some more to other parts of jedi (-vim), growing a bit tired with YouCompleteMe (both the slow pace of PR there, and the focus of the product on c-like languages). This means getting the http server to work for jedi (not sure if someone is working on that yet) and then making jedi-vim a bit more to my liking (I like the fuzzy-match and auto-activate in ycm). But that's fully off-topic :).

Typing module seems the right way to go next (interestingly enough I seem to have skipped over it completely in my todo-list). I was also looking at making the inline type-hints (# type: int) work, but as far as I can see, all comments get removed from the source somewhere early on and are not available in the parsed code anymore. Might be not too hard to fix, but actually the one use-case I had for it in my own code so far, would be solved by having a typing.List[X] working.

My first order of business would be to find a typing-module that will work on python 3.4. This seems to be suggested by Jukka's/MyPy's (claimed to be obsolete) typing module, but it doesn't actually work on 3.4 (result of 5 minutes google and playing, should definitely dive into it more in-depth).

Also not yet clear on what to do next. Probably make some list on to implement in what order, but will do that when I get there.

Contributor

reinhrst commented Dec 21, 2015

Thanks! It was certainly a nice experience to have a PR made and merged within a week :D. I can very much imagine I will want to contribute some more to other parts of jedi (-vim), growing a bit tired with YouCompleteMe (both the slow pace of PR there, and the focus of the product on c-like languages). This means getting the http server to work for jedi (not sure if someone is working on that yet) and then making jedi-vim a bit more to my liking (I like the fuzzy-match and auto-activate in ycm). But that's fully off-topic :).

Typing module seems the right way to go next (interestingly enough I seem to have skipped over it completely in my todo-list). I was also looking at making the inline type-hints (# type: int) work, but as far as I can see, all comments get removed from the source somewhere early on and are not available in the parsed code anymore. Might be not too hard to fix, but actually the one use-case I had for it in my own code so far, would be solved by having a typing.List[X] working.

My first order of business would be to find a typing-module that will work on python 3.4. This seems to be suggested by Jukka's/MyPy's (claimed to be obsolete) typing module, but it doesn't actually work on 3.4 (result of 5 minutes google and playing, should definitely dive into it more in-depth).

Also not yet clear on what to do next. Probably make some list on to implement in what order, but will do that when I get there.

@reinhrst reinhrst deleted the reinhrst:linter branch Dec 21, 2015

@davidhalter

This comment has been minimized.

Show comment
Hide comment
@davidhalter

davidhalter Dec 21, 2015

Owner

I was also looking at making the inline type-hints (# type: int) work, but as far as I can see, all comments get removed from the source somewhere early on and are not available in the parsed code anymore.

This is wrong. The comments are just hidden :-) They are part of the whole Leaf.prefix (which includes whitespace). You could simply implement a method Leaf.get_comments() that returns all the comments (excluding #). And then you do Function.children[suite_index].first_leaf().get_comments()[0].

This means getting the http server to work for jedi (not sure if someone is working on that yet) and then making jedi-vim a bit more to my liking (I like the fuzzy-match and auto-activate in ycm).

No one has seriously claimed this issue, yet. I also think these too issues are important and would very happily see them implemented - but I have other priorities within Jedi (type inference improvements).

My first order of business would be to find a typing-module that will work on python 3.4. This seems to be suggested by Jukka's/MyPy's (claimed to be obsolete) typing module, but it doesn't actually work on 3.4 (result of 5 minutes google and playing, should definitely dive into it more in-depth).

Why is the default module not working in Python 3.4 (because the one you're pointing at now is the Python 2 version)?

In my opinion it would be great to understand typing.py as well as possible without a lot of magic or hard coding. Because that's just way more fun than implementing it by hand :) As a starting point for the typing module we could try to evaluate the __extra__ attribute and use completion on it.

Owner

davidhalter commented Dec 21, 2015

I was also looking at making the inline type-hints (# type: int) work, but as far as I can see, all comments get removed from the source somewhere early on and are not available in the parsed code anymore.

This is wrong. The comments are just hidden :-) They are part of the whole Leaf.prefix (which includes whitespace). You could simply implement a method Leaf.get_comments() that returns all the comments (excluding #). And then you do Function.children[suite_index].first_leaf().get_comments()[0].

This means getting the http server to work for jedi (not sure if someone is working on that yet) and then making jedi-vim a bit more to my liking (I like the fuzzy-match and auto-activate in ycm).

No one has seriously claimed this issue, yet. I also think these too issues are important and would very happily see them implemented - but I have other priorities within Jedi (type inference improvements).

My first order of business would be to find a typing-module that will work on python 3.4. This seems to be suggested by Jukka's/MyPy's (claimed to be obsolete) typing module, but it doesn't actually work on 3.4 (result of 5 minutes google and playing, should definitely dive into it more in-depth).

Why is the default module not working in Python 3.4 (because the one you're pointing at now is the Python 2 version)?

In my opinion it would be great to understand typing.py as well as possible without a lot of magic or hard coding. Because that's just way more fun than implementing it by hand :) As a starting point for the typing module we could try to evaluate the __extra__ attribute and use completion on it.

@reinhrst

This comment has been minimized.

Show comment
Hide comment
@reinhrst

reinhrst Dec 22, 2015

Contributor

This is wrong. The comments are just hidden :-) They are part of the whole Leaf.prefix (which includes whitespace). You could simply implement a method Leaf.get_comments() that returns all the comments (excluding #). And then you do Function.children[suite_index].first_leaf().get_comments()[0].

Good to know. Looks like you're talking about pep-0484-like function annotation for python 2.x; I actually meant the a = 3 # type: str annotations. However same story, see new PR.

No one has seriously claimed this issue, yet. I also think these too issues are important and would very happily see them implemented - but I have other priorities within Jedi (type inference improvements).

Any chance you have some description lying about on either? I think I saw in jedi-vim repo that you would like to see an async jedi server (and I fully agree); however I suspect (without having done too much research) that building something async, for python 2.6-3.5, on all OSses, without importing something like twisted or tornado, might be a tall order.

Why is the default module not working in Python 3.4 (because the one you're pointing at now is the Python 2 version)?

Yuk, guess I was still sleeping this morning

In my opinion it would be great to understand typing.py as well as possible without a lot of magic or hard coding. Because that's just way more fun than implementing it by hand :) As a starting point for the typing module we could try to evaluate the __extra__ attribute and use completion on it.

Hey, I also missed this comment before I wrote my things in issue #663 . Haven't looked at __extra__ yet, will give it a look sometime soon.

Contributor

reinhrst commented Dec 22, 2015

This is wrong. The comments are just hidden :-) They are part of the whole Leaf.prefix (which includes whitespace). You could simply implement a method Leaf.get_comments() that returns all the comments (excluding #). And then you do Function.children[suite_index].first_leaf().get_comments()[0].

Good to know. Looks like you're talking about pep-0484-like function annotation for python 2.x; I actually meant the a = 3 # type: str annotations. However same story, see new PR.

No one has seriously claimed this issue, yet. I also think these too issues are important and would very happily see them implemented - but I have other priorities within Jedi (type inference improvements).

Any chance you have some description lying about on either? I think I saw in jedi-vim repo that you would like to see an async jedi server (and I fully agree); however I suspect (without having done too much research) that building something async, for python 2.6-3.5, on all OSses, without importing something like twisted or tornado, might be a tall order.

Why is the default module not working in Python 3.4 (because the one you're pointing at now is the Python 2 version)?

Yuk, guess I was still sleeping this morning

In my opinion it would be great to understand typing.py as well as possible without a lot of magic or hard coding. Because that's just way more fun than implementing it by hand :) As a starting point for the typing module we could try to evaluate the __extra__ attribute and use completion on it.

Hey, I also missed this comment before I wrote my things in issue #663 . Haven't looked at __extra__ yet, will give it a look sometime soon.

@davidhalter davidhalter referenced this pull request Dec 22, 2015

Merged

PEP 484 typing library #663

@davidhalter

This comment has been minimized.

Show comment
Hide comment
@davidhalter

davidhalter Dec 24, 2015

Owner

Any chance you have some description lying about on either? I think I saw in jedi-vim repo that you would like to see an async jedi server (and I fully agree); however I suspect (without having done too much research) that building something async, for python 2.6-3.5, on all OSses, without importing something like twisted or tornado, might be a tall order.

TBH I don't have a lot of experience with async servers. I would imagine it should be possible setting up tcp/unix sockets without a big hassle, but I might be completely wrong. In that case I would be happy to use tornado or twisted (or something similar).

Owner

davidhalter commented Dec 24, 2015

Any chance you have some description lying about on either? I think I saw in jedi-vim repo that you would like to see an async jedi server (and I fully agree); however I suspect (without having done too much research) that building something async, for python 2.6-3.5, on all OSses, without importing something like twisted or tornado, might be a tall order.

TBH I don't have a lot of experience with async servers. I would imagine it should be possible setting up tcp/unix sockets without a big hassle, but I might be completely wrong. In that case I would be happy to use tornado or twisted (or something similar).

@reinhrst

This comment has been minimized.

Show comment
Hide comment
@reinhrst

reinhrst Dec 24, 2015

Contributor

OK, looked around a bit, and it looks like the asyncore module does actually provide the building blocks one needs without having to deal with async sockets themselves in different OSses. Building a small http server on top should not be to hard (and probably already exists somewhere out there on the internet).

A question might actually be why one would care for an async server at all. Since jedi is 100% CPU bound (except getting the python files from disk, but I don't think we have to take that into account), what would the advantage of an async server be? I guess though that first we would have to consider what exactly the server should do, and how this should be done.

In my case the main goal in getting this to work, is to get jedi to be independent from whatever version of python I compiled into vim (and whatever virtualenv was active when I started vim). For this, all I require is that jedi runs in its own process I think (and that I can possibly start multiple jedi's if I'm editing 2 files in one vim instance that run on different versions of python / different virtualenvs --- obviously this raises all kind on new questions on how vim knows what python/virtualenv should be used for a particular file, but let's skip that for now).

Using a HTTP server on jedi introduces 2 layers of complexity that I'm not sure are needed at all:

  • HTTP uses a new connection per request. This means (in addition to minor extra overhead, both in CPU and in code complexity) that in theory someone could try connecting two different clients to the same server. Is this something we would want to support? E.g. if I'm sure that all connections are from one client, I know I can ignore an older request for completion when a new one comes in (I think). With multiple clients, no such assumption can be made. In addition, jedi-server has no way of knowing when it should shut down (if the client crashes and forgets to send a kill command). I recon it could decide to shut down after not seeing any requests for 10 minutes or so assuming the client will start a new server if it finds the old one dead, but this means that all cache is invalidated as soon as you take a small coffee break. Using a persistent connection, the OS is responsible for figuring out if the other side died, and jedi-server can just kill itself if the connection drops (or if, at startup, no connection was made within x seconds). Finally, HTTP itself introduces quite some code.
  • Using sockets does bring in a question of security. If no extra security measures are taken, I think that from a security perspective it should be assumed that anyone being able to connect to the socket, can read any file that the user the jedi-server is running under (perhaps not exactly in the current codebase, but you don't want to consider these security implications every time something is added to jedi). If jedi-server uses an TCP/IP socket on 127.0.0.1, any process on localhost (on normal desktop OSses) can access these. Alternatively a unix/NT-socket can be used, which one could make writable only to the correct user, which would solve most of these issues, but probably need extensive testing on different OSses (and I for one don't have a windows machine here). As a security precaution each request could be accompanied by a secret that is shared between client and server; not hard but certainly extra complexity. As a final alternative it's also an idea to say that the security issue is one we're willing to take (which seems to be what iPython notebooks do, except that there you also get write-permissions....).

The simplest solution in my eyes would be to just communicate to jedi-server over stdin/stdout (I'm not 100% sure that vim can do this, start a subprocess, communicate to it while being responsive to user interaction; I think this was one of the reasons why REPLs could be implemented in emacs but not in vim, but I'm not sure). Even if in some situations a full HTTP-interface would be needed, I think it would still make sense to also offer a much simpler interface over stdin.

So I think that step one would be to get a good idea of use cases. I can reason very well out of my own situation, not knowing which other situations jedi is being used in.

Contributor

reinhrst commented Dec 24, 2015

OK, looked around a bit, and it looks like the asyncore module does actually provide the building blocks one needs without having to deal with async sockets themselves in different OSses. Building a small http server on top should not be to hard (and probably already exists somewhere out there on the internet).

A question might actually be why one would care for an async server at all. Since jedi is 100% CPU bound (except getting the python files from disk, but I don't think we have to take that into account), what would the advantage of an async server be? I guess though that first we would have to consider what exactly the server should do, and how this should be done.

In my case the main goal in getting this to work, is to get jedi to be independent from whatever version of python I compiled into vim (and whatever virtualenv was active when I started vim). For this, all I require is that jedi runs in its own process I think (and that I can possibly start multiple jedi's if I'm editing 2 files in one vim instance that run on different versions of python / different virtualenvs --- obviously this raises all kind on new questions on how vim knows what python/virtualenv should be used for a particular file, but let's skip that for now).

Using a HTTP server on jedi introduces 2 layers of complexity that I'm not sure are needed at all:

  • HTTP uses a new connection per request. This means (in addition to minor extra overhead, both in CPU and in code complexity) that in theory someone could try connecting two different clients to the same server. Is this something we would want to support? E.g. if I'm sure that all connections are from one client, I know I can ignore an older request for completion when a new one comes in (I think). With multiple clients, no such assumption can be made. In addition, jedi-server has no way of knowing when it should shut down (if the client crashes and forgets to send a kill command). I recon it could decide to shut down after not seeing any requests for 10 minutes or so assuming the client will start a new server if it finds the old one dead, but this means that all cache is invalidated as soon as you take a small coffee break. Using a persistent connection, the OS is responsible for figuring out if the other side died, and jedi-server can just kill itself if the connection drops (or if, at startup, no connection was made within x seconds). Finally, HTTP itself introduces quite some code.
  • Using sockets does bring in a question of security. If no extra security measures are taken, I think that from a security perspective it should be assumed that anyone being able to connect to the socket, can read any file that the user the jedi-server is running under (perhaps not exactly in the current codebase, but you don't want to consider these security implications every time something is added to jedi). If jedi-server uses an TCP/IP socket on 127.0.0.1, any process on localhost (on normal desktop OSses) can access these. Alternatively a unix/NT-socket can be used, which one could make writable only to the correct user, which would solve most of these issues, but probably need extensive testing on different OSses (and I for one don't have a windows machine here). As a security precaution each request could be accompanied by a secret that is shared between client and server; not hard but certainly extra complexity. As a final alternative it's also an idea to say that the security issue is one we're willing to take (which seems to be what iPython notebooks do, except that there you also get write-permissions....).

The simplest solution in my eyes would be to just communicate to jedi-server over stdin/stdout (I'm not 100% sure that vim can do this, start a subprocess, communicate to it while being responsive to user interaction; I think this was one of the reasons why REPLs could be implemented in emacs but not in vim, but I'm not sure). Even if in some situations a full HTTP-interface would be needed, I think it would still make sense to also offer a much simpler interface over stdin.

So I think that step one would be to get a good idea of use cases. I can reason very well out of my own situation, not knowing which other situations jedi is being used in.

@davidhalter

This comment has been minimized.

Show comment
Hide comment
@davidhalter

davidhalter Dec 24, 2015

Owner

what would the advantage of an async server be?

That's a good question. Let me rephrase my thoughts about "async": I used the words async, because I want asynchronous autocompletion (e.g. in jedi-vim). However for that the server doesn't have to do crazy stuff. Just queueing up the requests will be enough!

In my case the main goal in getting this to work, is to get jedi to be independent from whatever version of python I compiled into vim (and whatever virtualenv was active when I started vim). For this, all I require is that jedi runs in its own process I think (and that I can possibly start multiple jedi's if I'm editing 2 files in one vim instance that run on different versions of python / different virtualenvs

THIS. Exactly. This is all we need.

The simplest solution in my eyes would be to just communicate to jedi-server over stdin/stdout

That would indeed be the simplest solution. However I'm not sure if it's going to be good enough?! Once I have thought about just having one server per Python version (to avoid issues with parser file caching), but do we really need that? I'm not so sure. Probably it's just way easier to do it the way you proposed. (Also for virtualenvs). We can still find another way to avoid files clashes.

I think your approach seems to be very solid. If you want, you can implement it. Given my lack of experience in this area I'm very happy if someone helps me out.

@dbrgn What do you think? HTTP Server vs. TCP/IP sockets vs. unix sockets vs...

Owner

davidhalter commented Dec 24, 2015

what would the advantage of an async server be?

That's a good question. Let me rephrase my thoughts about "async": I used the words async, because I want asynchronous autocompletion (e.g. in jedi-vim). However for that the server doesn't have to do crazy stuff. Just queueing up the requests will be enough!

In my case the main goal in getting this to work, is to get jedi to be independent from whatever version of python I compiled into vim (and whatever virtualenv was active when I started vim). For this, all I require is that jedi runs in its own process I think (and that I can possibly start multiple jedi's if I'm editing 2 files in one vim instance that run on different versions of python / different virtualenvs

THIS. Exactly. This is all we need.

The simplest solution in my eyes would be to just communicate to jedi-server over stdin/stdout

That would indeed be the simplest solution. However I'm not sure if it's going to be good enough?! Once I have thought about just having one server per Python version (to avoid issues with parser file caching), but do we really need that? I'm not so sure. Probably it's just way easier to do it the way you proposed. (Also for virtualenvs). We can still find another way to avoid files clashes.

I think your approach seems to be very solid. If you want, you can implement it. Given my lack of experience in this area I'm very happy if someone helps me out.

@dbrgn What do you think? HTTP Server vs. TCP/IP sockets vs. unix sockets vs...

@dbrgn

This comment has been minimized.

Show comment
Hide comment
@dbrgn

dbrgn Dec 27, 2015

Collaborator

YCM by @Valloric uses a HTTP+JSON server model, see https://github.com/Valloric/YouCompleteMe#client-server-architecture.

More info:

@jwilm is currently working on a YCM plugin for Rust with the same API: Valloric/ycmd#268

Maybe you can use the same API? Would probably make YCM integration better too. Sometimes the Jedi process crashes and then you have to restart vim for the completion to work again.

Collaborator

dbrgn commented Dec 27, 2015

YCM by @Valloric uses a HTTP+JSON server model, see https://github.com/Valloric/YouCompleteMe#client-server-architecture.

More info:

@jwilm is currently working on a YCM plugin for Rust with the same API: Valloric/ycmd#268

Maybe you can use the same API? Would probably make YCM integration better too. Sometimes the Jedi process crashes and then you have to restart vim for the completion to work again.

@reinhrst

This comment has been minimized.

Show comment
Hide comment
@reinhrst

reinhrst Dec 27, 2015

Contributor

I've also been involved in the ycmd-jedi HTTP effort at Valloric/ycmd#246 . There was some discussion there as well over what path to take, it was postponed to later. Especially since there was already a working implementation for HTTP on TCP sockets.

Here we don't have such a problem, and can do it the right way straight from the start.

Part of my issues with HTTP come from my experience with ycm. There is a lot of code needed to implement the start/stop server, and I always have a whole bunch of orphaned jedi-http processes in my process list that don't die (I'm running the PR in Valloric/ycmd#246 ).

Contributor

reinhrst commented Dec 27, 2015

I've also been involved in the ycmd-jedi HTTP effort at Valloric/ycmd#246 . There was some discussion there as well over what path to take, it was postponed to later. Especially since there was already a working implementation for HTTP on TCP sockets.

Here we don't have such a problem, and can do it the right way straight from the start.

Part of my issues with HTTP come from my experience with ycm. There is a lot of code needed to implement the start/stop server, and I always have a whole bunch of orphaned jedi-http processes in my process list that don't die (I'm running the PR in Valloric/ycmd#246 ).

@Valloric

This comment has been minimized.

Show comment
Hide comment
@Valloric

Valloric Dec 27, 2015

Note that an HTTP server wrapping Jedi is already being written by a YCM contributor: https://github.com/vheon/JediHTTP

@vheon

Valloric commented Dec 27, 2015

Note that an HTTP server wrapping Jedi is already being written by a YCM contributor: https://github.com/vheon/JediHTTP

@vheon

@vheon

This comment has been minimized.

Show comment
Hide comment
@vheon

vheon Dec 27, 2015

Part of my issues with HTTP come from my experience with ycm. There is a lot of code needed to implement the start/stop server, and I always have a whole bunch of orphaned jedi-http processes in my process list that don't die (I'm running the PR in Valloric/ycmd#246 ).

A comment on the PR could've be of help.

Using a HTTP server on jedi introduces 2 layers of complexity that I'm not sure are needed at all:

  • HTTP uses a new connection per request. This means (in addition to minor extra overhead, both in CPU and in code complexity) that in theory someone could try connecting two different clients to the same server. Is this something we would want to support? E.g. if I'm sure that all connections are from one client, I know I can ignore an older request for completion when a new one comes in (I think). With multiple clients, no such assumption can be made. In addition, jedi-server has no way of knowing when it should shut down (if the client crashes and forgets to send a kill command). I recon it could decide to shut down after not seeing any requests for 10 minutes or so assuming the client will start a new server if it finds the old one dead, but this means that all cache is invalidated as soon as you take a small coffee break. Using a persistent connection, the OS is responsible for figuring out if the other side died, and jedi-server can just kill itself if the connection drops (or if, at startup, no connection was made within x seconds). Finally, HTTP itself introduces quite some code.
  • Using sockets does bring in a question of security. If no extra security measures are taken, I think that from a security perspective it should be assumed that anyone being able to connect to the socket, can read any file that the user the jedi-server is running under (perhaps not exactly in the current codebase, but you don't want to consider these security implications every time something is added to jedi). If jedi-server uses an TCP/IP socket on 127.0.0.1, any process on localhost (on normal desktop OSses) can access these. Alternatively a unix/NT-socket can be used, which one could make writable only to the correct user, which would solve most of these issues, but probably need extensive testing on different OSses (and I for one don't have a windows machine here). As a security precaution each request could be accompanied by a secret that is shared between client and server; not hard but certainly extra complexity. As a final alternative it's also an idea to say that the security issue is one we're willing to take (which seems to be what iPython notebooks do, except that there you also get write-permissions....).

On vheon/JediHTTP you can see all the code needed to implement the HTTP layer (is really small) and using HMAC as the authentication method resolve the issues that you have I believe. Just yesterday I was looking at the problem that you brought up in Valloric/ycmd#246 (https://github.com/reinhrst/jedi-working-directory-example) but it doesn't seem to work with the current implementation of ycmd either.

vheon commented Dec 27, 2015

Part of my issues with HTTP come from my experience with ycm. There is a lot of code needed to implement the start/stop server, and I always have a whole bunch of orphaned jedi-http processes in my process list that don't die (I'm running the PR in Valloric/ycmd#246 ).

A comment on the PR could've be of help.

Using a HTTP server on jedi introduces 2 layers of complexity that I'm not sure are needed at all:

  • HTTP uses a new connection per request. This means (in addition to minor extra overhead, both in CPU and in code complexity) that in theory someone could try connecting two different clients to the same server. Is this something we would want to support? E.g. if I'm sure that all connections are from one client, I know I can ignore an older request for completion when a new one comes in (I think). With multiple clients, no such assumption can be made. In addition, jedi-server has no way of knowing when it should shut down (if the client crashes and forgets to send a kill command). I recon it could decide to shut down after not seeing any requests for 10 minutes or so assuming the client will start a new server if it finds the old one dead, but this means that all cache is invalidated as soon as you take a small coffee break. Using a persistent connection, the OS is responsible for figuring out if the other side died, and jedi-server can just kill itself if the connection drops (or if, at startup, no connection was made within x seconds). Finally, HTTP itself introduces quite some code.
  • Using sockets does bring in a question of security. If no extra security measures are taken, I think that from a security perspective it should be assumed that anyone being able to connect to the socket, can read any file that the user the jedi-server is running under (perhaps not exactly in the current codebase, but you don't want to consider these security implications every time something is added to jedi). If jedi-server uses an TCP/IP socket on 127.0.0.1, any process on localhost (on normal desktop OSses) can access these. Alternatively a unix/NT-socket can be used, which one could make writable only to the correct user, which would solve most of these issues, but probably need extensive testing on different OSses (and I for one don't have a windows machine here). As a security precaution each request could be accompanied by a secret that is shared between client and server; not hard but certainly extra complexity. As a final alternative it's also an idea to say that the security issue is one we're willing to take (which seems to be what iPython notebooks do, except that there you also get write-permissions....).

On vheon/JediHTTP you can see all the code needed to implement the HTTP layer (is really small) and using HMAC as the authentication method resolve the issues that you have I believe. Just yesterday I was looking at the problem that you brought up in Valloric/ycmd#246 (https://github.com/reinhrst/jedi-working-directory-example) but it doesn't seem to work with the current implementation of ycmd either.

@davidhalter

This comment has been minimized.

Show comment
Hide comment
@davidhalter

davidhalter Dec 27, 2015

Owner

@reinhrst I would agree that stdin/stdout would probably be the easiest way. What I would like though still is some kind of threading. It doesn't need to be async, but the library should be able to use a callback, when e.g. completions are finished. Do you understand what I mean? Essentially this is the problem we're now facing in Jedi itself.

@vheon I'm very sceptical towards HTTP. What benefit does it bring in comparison to sockets and stdin/stdout?

Owner

davidhalter commented Dec 27, 2015

@reinhrst I would agree that stdin/stdout would probably be the easiest way. What I would like though still is some kind of threading. It doesn't need to be async, but the library should be able to use a callback, when e.g. completions are finished. Do you understand what I mean? Essentially this is the problem we're now facing in Jedi itself.

@vheon I'm very sceptical towards HTTP. What benefit does it bring in comparison to sockets and stdin/stdout?

@vheon

This comment has been minimized.

Show comment
Hide comment
@vheon

vheon Dec 27, 2015

@davidhalter can I reverse the question? What benefits does sockets or stdin/stdout bring?

vheon commented Dec 27, 2015

@davidhalter can I reverse the question? What benefits does sockets or stdin/stdout bring?

@davidhalter

This comment has been minimized.

Show comment
Hide comment
@davidhalter

davidhalter Dec 27, 2015

Owner

@vheon Talking about stdin/stdout: It's very simple and secure. HTTP seems to be a strange choice when all you want is IPC.

Apart from that it's really hard to do session management in HTTP. With stdin/out you cannot go wrong, really.

Owner

davidhalter commented Dec 27, 2015

@vheon Talking about stdin/stdout: It's very simple and secure. HTTP seems to be a strange choice when all you want is IPC.

Apart from that it's really hard to do session management in HTTP. With stdin/out you cannot go wrong, really.

@vheon

This comment has been minimized.

Show comment
Hide comment
@vheon

vheon Dec 27, 2015

I'm not thinking this though but why do I want to do session management? I mean, the way I see it the user tell you: "this is my current file path, its unsaved content is this, the cursor is at position <x,y>, what completions are available here? I don't see much session here, but probably I'm thinking of a completely different thing than you :(

If I can ask you something though:

What I would like though still is some kind of threading.

How can we build some king of threading if jedi itself is not thread safe? Let's say that I've got some sort of IPC with jedi and ask for possible completion to two different jedi processes. Would that work? is the cache shared in some sort? From the doc I gathered that the cache is one of the problem of the non thread safeness of jedi so I'm just guessing here.

vheon commented Dec 27, 2015

I'm not thinking this though but why do I want to do session management? I mean, the way I see it the user tell you: "this is my current file path, its unsaved content is this, the cursor is at position <x,y>, what completions are available here? I don't see much session here, but probably I'm thinking of a completely different thing than you :(

If I can ask you something though:

What I would like though still is some kind of threading.

How can we build some king of threading if jedi itself is not thread safe? Let's say that I've got some sort of IPC with jedi and ask for possible completion to two different jedi processes. Would that work? is the cache shared in some sort? From the doc I gathered that the cache is one of the problem of the non thread safeness of jedi so I'm just guessing here.

@reinhrst

This comment has been minimized.

Show comment
Hide comment
@reinhrst

reinhrst Dec 27, 2015

Contributor

@vheon

A comment on the PR could've be of help.

True.. It's on the "I need to dive a bit deeper into this to actually find out of it's not my own stupid fault before reporting it"-list... But it might have been nice to send a heads-up in the meantime, sorry....

On vheon/JediHTTP you can see all the code needed to implement the HTTP layer (is really small)...

I think you're using waitress/bottle as dependencies, right? It's probably not a show-stopper, but would be nice if this could be fully selfcontained, I think. And HMAC does solve the problem with security, but it's just extra complexity that needs extra tests to check if it works, extra complexity for anyone writing a client against jedi, and possible security risc if the person writing the client does not choose or protect his shared secrets. It's all solvable, the question is if it's worth the trouble. (On the other hand, using HTTP would possibly make things easier for an client, compared to a custom protocol on the wire that needs delimitation, etc).

@davidhalter

What I would like though still is some kind of threading....

So I think what you would want is the client to fire off a completion request, do something else in the meantime (i.e. be responsive to the user), then get the result of the completion request when it's done and then show the result to the user (if it still makes sense and the user isn't on another word by now, right?). All this just means that the client needs to be async (i.e. doing something else while jedi is working). Now in plain python this is easy, regardless of stdin/out, sockets direct connection or HTTP. Question is whether something like vim can do it (I know very little about vim, but I seem to remember that it doesn't like having to keep background processes / other threads alive while offering user interaction). If that's true, that limits the options, and we'd need something like HTTP, where a user presses os.p, vim then fires off an HTTP request to jedi asking for completion, closes the connection, and after 100ms does another HTTP request asking jedi if it has an answer already, and possibly another if there is still no answer yet. I guess I need to figure this out first before the rest of the discussion makes any sense.

@davidhalter & @vheon
Just to be clear, there are 2 choices: connection (stdout / unix (or NT) sockets or TCP port). stdin/out is probably easiest, TCP port will definitely need HMAC, unix sockets probably need HMAC (or one might circumvent security policies on SELinux, I think, possibly ;) )

Second choice is protocol: one connection, simple protocol (only choice for stdin/out). Easy for jedi to find out that it needs to stop since the client died. Or HTTP (-like) multi-connection (sure, can be one connection through keep-alive, but a broken connection doesn't mean anything, and a single client may (probably will) make multiple connections belong to the same person), having the advantage that a client will not have to keep a connection open.

So I think we might want to postpone this discussion until after (I, someone), finds out what the requirement is on the vim side.

Contributor

reinhrst commented Dec 27, 2015

@vheon

A comment on the PR could've be of help.

True.. It's on the "I need to dive a bit deeper into this to actually find out of it's not my own stupid fault before reporting it"-list... But it might have been nice to send a heads-up in the meantime, sorry....

On vheon/JediHTTP you can see all the code needed to implement the HTTP layer (is really small)...

I think you're using waitress/bottle as dependencies, right? It's probably not a show-stopper, but would be nice if this could be fully selfcontained, I think. And HMAC does solve the problem with security, but it's just extra complexity that needs extra tests to check if it works, extra complexity for anyone writing a client against jedi, and possible security risc if the person writing the client does not choose or protect his shared secrets. It's all solvable, the question is if it's worth the trouble. (On the other hand, using HTTP would possibly make things easier for an client, compared to a custom protocol on the wire that needs delimitation, etc).

@davidhalter

What I would like though still is some kind of threading....

So I think what you would want is the client to fire off a completion request, do something else in the meantime (i.e. be responsive to the user), then get the result of the completion request when it's done and then show the result to the user (if it still makes sense and the user isn't on another word by now, right?). All this just means that the client needs to be async (i.e. doing something else while jedi is working). Now in plain python this is easy, regardless of stdin/out, sockets direct connection or HTTP. Question is whether something like vim can do it (I know very little about vim, but I seem to remember that it doesn't like having to keep background processes / other threads alive while offering user interaction). If that's true, that limits the options, and we'd need something like HTTP, where a user presses os.p, vim then fires off an HTTP request to jedi asking for completion, closes the connection, and after 100ms does another HTTP request asking jedi if it has an answer already, and possibly another if there is still no answer yet. I guess I need to figure this out first before the rest of the discussion makes any sense.

@davidhalter & @vheon
Just to be clear, there are 2 choices: connection (stdout / unix (or NT) sockets or TCP port). stdin/out is probably easiest, TCP port will definitely need HMAC, unix sockets probably need HMAC (or one might circumvent security policies on SELinux, I think, possibly ;) )

Second choice is protocol: one connection, simple protocol (only choice for stdin/out). Easy for jedi to find out that it needs to stop since the client died. Or HTTP (-like) multi-connection (sure, can be one connection through keep-alive, but a broken connection doesn't mean anything, and a single client may (probably will) make multiple connections belong to the same person), having the advantage that a client will not have to keep a connection open.

So I think we might want to postpone this discussion until after (I, someone), finds out what the requirement is on the vim side.

@vheon

This comment has been minimized.

Show comment
Hide comment
@vheon

vheon Dec 28, 2015

(On the other hand, using HTTP would possibly make things easier for an client, compared to a custom protocol on the wire that needs delimitation, etc).

That is exactly why we choose HTTP as the delivery mechanism: every language has some sort of HTTP library.

Question is whether something like vim can do it (I know very little about vim, but I seem to remember that it doesn't like having to keep background processes / other threads alive while offering user interaction). If that's true, that limits the options, and we'd need something like HTTP, where a user presses os.p, vim then fires off an HTTP request to jedi asking for completion, closes the connection, and after 100ms does another HTTP request asking jedi if it has an answer already, and possibly another if there is still no answer yet. I guess I need to figure this out first before the rest of the discussion makes any sense.

That is exactly what the YCM client do.

vheon commented Dec 28, 2015

(On the other hand, using HTTP would possibly make things easier for an client, compared to a custom protocol on the wire that needs delimitation, etc).

That is exactly why we choose HTTP as the delivery mechanism: every language has some sort of HTTP library.

Question is whether something like vim can do it (I know very little about vim, but I seem to remember that it doesn't like having to keep background processes / other threads alive while offering user interaction). If that's true, that limits the options, and we'd need something like HTTP, where a user presses os.p, vim then fires off an HTTP request to jedi asking for completion, closes the connection, and after 100ms does another HTTP request asking jedi if it has an answer already, and possibly another if there is still no answer yet. I guess I need to figure this out first before the rest of the discussion makes any sense.

That is exactly what the YCM client do.

@davidhalter

This comment has been minimized.

Show comment
Hide comment
@davidhalter

davidhalter Dec 28, 2015

Owner

@reinhrst

So I think what you would want is the client to fire off a completion request, do something else in the meantime. [..]

Exactly!

Question is whether something like vim can do it

Well, I think we have to go farther than Jedi. It's basically every client that doesn't have a Python interface. jedi-vim/ycm are only a very small subset of plugins that are using Jedi. I think therefore it would be really interesting to create something that works for multiple languages.

I was just thinking about the unix philosophy: Work with text.
We could still do it with stdin/stdout and provide a proper protocol that people could use from other languages as well. Having done that, we can write a Python client on top of that, that supports threading. (For VIM we might need to do something different, but


Just for sorting things with VIM: I just tested Python threads. They seem to work pretty well. I don't know how it works, but it does. Without Thread.setDaemon(True) you will have trouble ending vim. The only weird thing that happened with this script:

import time
import threading


class Thread(threading.Thread):
    def run(self):
        while True:
            print('REPORT')
            time.sleep(0.5)


t = Thread()
t.setDaemon(True)                    
t.start()

time.sleep(1.5)
print('DONE')

Was this error here:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: No module named py

@vheon
Well, it's true that alomst all languages have an HTTP client, but this does not mean that they support your special protocol. So there's almost no gain there. Also each language would have to painfully recreate your HMAC implementation, whereas in stdin/stdout a lot of things are not an issue anymore.

Question is whether something like vim can do it (I know very little about vim, but I seem to remember that it doesn't like having to keep background processes / other threads alive while offering user interaction). If that's true, that limits the options, and we'd need something like HTTP, where a user presses os.p, vim then fires off an HTTP request to jedi asking for completion, closes the connection, and after 100ms does another HTTP request asking jedi if it has an answer already, and possibly another if there is still no answer yet. I guess I need to figure this out first before the rest of the discussion makes any sense.

That is exactly what the YCM client do.

And how is this now better than simple stdin polling? What are the advantages? You still have to convince me.

Owner

davidhalter commented Dec 28, 2015

@reinhrst

So I think what you would want is the client to fire off a completion request, do something else in the meantime. [..]

Exactly!

Question is whether something like vim can do it

Well, I think we have to go farther than Jedi. It's basically every client that doesn't have a Python interface. jedi-vim/ycm are only a very small subset of plugins that are using Jedi. I think therefore it would be really interesting to create something that works for multiple languages.

I was just thinking about the unix philosophy: Work with text.
We could still do it with stdin/stdout and provide a proper protocol that people could use from other languages as well. Having done that, we can write a Python client on top of that, that supports threading. (For VIM we might need to do something different, but


Just for sorting things with VIM: I just tested Python threads. They seem to work pretty well. I don't know how it works, but it does. Without Thread.setDaemon(True) you will have trouble ending vim. The only weird thing that happened with this script:

import time
import threading


class Thread(threading.Thread):
    def run(self):
        while True:
            print('REPORT')
            time.sleep(0.5)


t = Thread()
t.setDaemon(True)                    
t.start()

time.sleep(1.5)
print('DONE')

Was this error here:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: No module named py

@vheon
Well, it's true that alomst all languages have an HTTP client, but this does not mean that they support your special protocol. So there's almost no gain there. Also each language would have to painfully recreate your HMAC implementation, whereas in stdin/stdout a lot of things are not an issue anymore.

Question is whether something like vim can do it (I know very little about vim, but I seem to remember that it doesn't like having to keep background processes / other threads alive while offering user interaction). If that's true, that limits the options, and we'd need something like HTTP, where a user presses os.p, vim then fires off an HTTP request to jedi asking for completion, closes the connection, and after 100ms does another HTTP request asking jedi if it has an answer already, and possibly another if there is still no answer yet. I guess I need to figure this out first before the rest of the discussion makes any sense.

That is exactly what the YCM client do.

And how is this now better than simple stdin polling? What are the advantages? You still have to convince me.

@vheon

This comment has been minimized.

Show comment
Hide comment
@vheon

vheon Dec 28, 2015

but this does not mean that they support your special protocol

which special protocol are you talking about? you mean the HMAC authentication?

vheon commented Dec 28, 2015

but this does not mean that they support your special protocol

which special protocol are you talking about? you mean the HMAC authentication?

@davidhalter

This comment has been minimized.

Show comment
Hide comment
@davidhalter

davidhalter Dec 28, 2015

Owner

which special protocol are you talking about? you mean the HMAC authentication?

I'm talking about the fact that by using HTTP there's now a way to communicate, but HTTP doesn't solve session management and so on. So you have to invent rules like use DELETE once you're done, use GET with a certain configuration for this and that and so on. The return values will maybe be JSON or something similar and so on. HTTP is not the solution, what you do on top of it is the actual work.

Owner

davidhalter commented Dec 28, 2015

which special protocol are you talking about? you mean the HMAC authentication?

I'm talking about the fact that by using HTTP there's now a way to communicate, but HTTP doesn't solve session management and so on. So you have to invent rules like use DELETE once you're done, use GET with a certain configuration for this and that and so on. The return values will maybe be JSON or something similar and so on. HTTP is not the solution, what you do on top of it is the actual work.

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