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

Remi behind jupyter server proxy #475

Merged
merged 20 commits into from Nov 26, 2021
Merged

Remi behind jupyter server proxy #475

merged 20 commits into from Nov 26, 2021

Conversation

gbrault
Copy link
Contributor

@gbrault gbrault commented Nov 22, 2021

  • Designed with minimal remi change in mind (see diff files to estimate the level of change)
  • For both remi core and remi apps (minimal change)
  • Fully tested with jupyter-server-proxy
  • Jupyter notebooks provided
  • works only with 3.6+ (f"" string interpolation)
  • If proxy not used, legacy remi app are working
  • adding proxy feature need some remi app rework (to calculate url for the front-end)
"url('/editor_resources:resize.png')" => f"url({_proxy.set_url('/editor_resources:resize.png')})"
# if directly provided to front-end
# if url are passed to Image or the like, no change

@dddomodossola
Copy link
Collaborator

@gbrault this is an extraordinary work! The work that can be done only by an experienced man. Please EXCUSE ME for the late reply.
I see f strings and Proxy a bit invasive.

Fstrings are fine for performances and readability, but it prevents the use on older python versions. Just to make you an example, I'm using remi on a plc (beckhoff softplc) that runs only python 3.4. Unfortunately this happens.

Proxy seems a bit invasive, I'm not sure to understand why it is required. Can you please explain it simply to me?

@gbrault
Copy link
Contributor Author

gbrault commented Nov 22, 2021

@dddomodossola I understand your point! I am interested by remi as a gui component for Jupyter based applications. I use Jupyter for processing data from production and sales for a medium company and time to time, I need some UI to enter data or browse sql database. Remi seems a good candidate for that.
Of course I understand my goal is not fully aligned with yours for making embedded application.
Maybe the gap implies to have 2 different version for remi?

@gbrault
Copy link
Contributor Author

gbrault commented Nov 22, 2021

Of course, I need a proxy to have both, the notebook and the UI interoperating seemlessly. The user see it as a single application. Starting the UI from a notebook enable to share the same python machine.

@dddomodossola
Copy link
Collaborator

@dddomodossola I understand your point! I am interested by remi as a gui component for Jupyter based applications. I use Jupyter for processing data from production and sales for a medium company and time to time, I need some UI to enter data or browse sql database. Remi seems a good candidate for that. Of course I understand my goal is not fully aligned with yours for making embedded application. Maybe the gap implies to have 2 different version for remi?

I'm really interested in your changes, lots of people uses jupyter and of course it is wonderful to get remi usable from it. Maybe this is feasible without affecting remi server. I have to study your changes in detail.

Of course, I need a proxy to have both, the notebook and the UI interoperating seemlessly. The user see it as a single application. Starting the UI from a notebook enable to share the same python machine.

Maybe the Proxy class can be avoided (not sure). I'm thinking about solving the resource problem directly in remi server:

  1. We can leave all the urls as is, i.e. "/res:myicon.png"
  2. At application runtime, the resource request arrives to the server as a GET request. Normally the server loads and sends the resource content to the client. In case of a jupyter app, the server _get_static_file function gets overridden, to load/redirect the request to jupyter. The function to override is just App._get_static_file.

If this solution could be feasible, the remi library could get untouched, and all gets solved by a template App, with a special _get_static_file function.

import remi.gui as gui
from remi import start, App
import os

class MyApp(App):
    def _get_static_file(self, filename):
        #here the magic should be done
        ...

    def idle(self):
        ...

    def main(self):
        ...


if __name__ == "__main__":
    start(MyApp)

Do you think this could be feasible? Thank you a lot for the collaboration.

@gbrault
Copy link
Contributor Author

gbrault commented Nov 23, 2021

This is a good idea!

import remi.gui as gui
from remi import start, App
import os

class MyApp(App):
    def _get_static_file(self, filename):
        #here the magic should be done
        ...

    def idle(self):
        ...

    def main(self):
        ...


if __name__ == "__main__":
    start(MyApp)

This solve all the cases where the client ask for a file and in _get_static_file, we have a chance to tweak all content where you have patterns like "/:" where res can be in ['res', "editor_resource', etc...]

I can give a try to write "#here the magic should be done"... (it should be mainly based on some code I wrote already)

I let you know.

@gbrault
Copy link
Contributor Author

gbrault commented Nov 23, 2021

@dddomodossola, I used your recommendation, it comes to only modifying server.py

  1. adding _net_interface_ip function to App class to be able to overload it (this is for the ws(s) address)
  2. creating an _overload function to App class to be able to overload it (to replace /res: with /proxy/8085/res:, not restricted to res)
  3. adding **kwards in the prototyp of __process_all
  4. Performing a call to _overload in every line of code (of App) which return html content to (to replace /res: with /proxy/8085/res:, not restricted to res)

I don't think I have used coding rules which cannot be used in earlier version of Python in server.py.

I calling code, it can be post 3.6+ version (no influence on your code)

If you want we can make a video call to show you it works and see if we can even simplify again!

Let me know!

@gbrault
Copy link
Contributor Author

gbrault commented Nov 23, 2021

I have also created all_paths to get all the /res: like tokens

@gbrault
Copy link
Contributor Author

gbrault commented Nov 23, 2021

One drawback of this method might be performance:

  • instead of calculating the right url (like in the Proxy version), it is done on the whole content for each request
  • this can be "time consuming" on small targets
  • however, if the replacement is not mad, we only add one call per content request, which is not much

@gbrault
Copy link
Contributor Author

gbrault commented Nov 24, 2021

Using IFrame, it works in a Notebook cell!
image

@gbrault
Copy link
Contributor Author

gbrault commented Nov 24, 2021

image

@dddomodossola
Copy link
Collaborator

@gbrault you did the magic! I gave a rapid look at the code and seems:

  1. Readable
  2. Simple
  3. Non invasive

I will analyze it better this late evening and merge. Thank you so much!

remi/server.py Outdated
self._log.debug('get: %s' % func)
if 'overload' not in kwargs:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is it required to pass the overload function as arguments? The App class is inherited, so the function _overload is already overloaded. Doesn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are perfectly right, this is not needed, we just need to use self._overload to call the function (it is a scorie which need to be cleaned-up: I make the correction and send new a new version.

@dddomodossola
Copy link
Collaborator

Why you preferred to replace/correct paths instead of resolving them by overloading the _get_static_file function? I supposed that overloading that _get_static_file function you can interpret the path inside it. Isn't this possible?

@gbrault
Copy link
Contributor Author

gbrault commented Nov 26, 2021

From the reverse proxy perspective (Jupyter-Server-Proxy), an url targeting the remi server, in the client (browser) needs to be

http(s)://<the server domain>:{jlabport}/proxy/{remiport}/<original path to remi server>

When the 'original' remi generates url, the /proxy/{remiport}/ part does not exists and it would rather send to the client

http(s)://<the server domain>:{remiport}/<original path to remi server>

From my understanding _get_static_file function would be usefull to find the os disk path if given

/proxy/{remiport}/<original path to remi server>

But this never happens, as the reverse proxy when called by the client with

http(s)://<the server domain>:{jlabport}/proxy/{remiport}/<original path to remi server>

calls back remi server with

http://localhost:{remiport}/<original path to remi server>

Do you agree, or am I missing something?

@dddomodossola
Copy link
Collaborator

Yes, thank you for the clarification.
Can you please also explain the code comment above?

@dddomodossola
Copy link
Collaborator

Thank you ;-)

@gbrault
Copy link
Contributor Author

gbrault commented Nov 26, 2021

The extra code in _process_all is cleaned

@dddomodossola dddomodossola merged commit 258d2a7 into rawpython:master Nov 26, 2021
@dddomodossola
Copy link
Collaborator

Merged, thank you 😊

@gbrault
Copy link
Contributor Author

gbrault commented Nov 26, 2021

You welcome!

@dddomodossola
Copy link
Collaborator

Now remi can be executed from Jupyter, this is cool

@gbrault
Copy link
Contributor Author

gbrault commented Nov 26, 2021

With this feature, when you have target platform able to support Jupyter Lab (I used it on a raspberry PI 3 A+ with success), you have a complete web access with:

  • remi as the UI
  • Notebooks
  • Jlab as the IDE (including a Terminal)

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