pythonic-emacs / anaconda-mode

Code navigation, documentation lookup and completion for Python.
GNU General Public License v3.0
707 stars 87 forks source link

Fix Basic RPC Tunnel with Tramp, and Enhance Tunnelling for Multihop Usage #357

Closed falloutphil closed 5 years ago

falloutphil commented 5 years ago

Fixes #335 and provides further enhancements as per requested in the issue comments

proofit404 commented 5 years ago

Hi! Thank you for all the time you invested in the resolution of this issue!

Unfortunately, I'm unable to review this PR due to the lack of understanding of how it works.

Can you explain the actual situation, the desired behavior and how this patch implements it?

I know there is a long conversation in the related issue. Unfortunately, I'm short on time so I can't read it from cover to cover and restore the whole picture from it chronologically.

Thanks for your time and your patience!

Regards, Artem.

falloutphil commented 5 years ago

Hi Artem,

Happy to help - I'm a big fan of the package!

Summary below:

My only concern is that the Docker use (rather than TRAMP/SSH use) remains compatible with the changes. I've highlighted as a PR code-review comment the one place that might need to handle Docker differently to TRAMP. When sending RPC requests via our local port forward - I've updated the http address to use the locally created tunnel - I suspect a predicate may be required to use the original http address with Docker. This is easy to fix if needs be... Alas I have no way of testing Docker use - so was hoping someone could confirm the RPC address in the Docker case?

proofit404 commented 5 years ago

Ok, cool!

If someone can test docker in this case and provides some logs here I'll be happy to merge PR.

Thanks for your time!

Regards, Artem.

AlexLewandowski commented 5 years ago

I don't use docker much, but I was able to get completion working from the Tensorflow docker image after installing socat. Not sure what else to report, but let me know if you need any details.

proofit404 commented 5 years ago

Hi @AlexLewandowski

Do you install anaconda-mode from the git branch of this PR?

Just to be clear.

Regards, Artem.

AlexLewandowski commented 5 years ago

Yes, I am using falloutphil:master to multi-hop behind a university server and also to test the docker case. In both cases, everything works well.

Couple notes on this pull request. The message "Anaconda Jump Proxy: nil" is printed. Is this supposed to be more informative?

As stated in #335, the multihop connections must be added (via ssh -J p1,p2 user@remote) to known_hosts. However, make sure that the connections were not previously added to known_hosts with aliases. This initially caused me some trouble. For those who have many servers that they access in different ways, you may want to save the hassle and delete your known_hosts.

proofit404 commented 5 years ago

Thank everyone involved in the solution to this issue!

proofit404 commented 5 years ago

Unfortunately, I was forced to revert it.

See https://github.com/proofit404/anaconda-mode/issues/358