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

Feature request: modify server root directory #430

Open
batzkass opened this issue Feb 10, 2021 · 22 comments
Open

Feature request: modify server root directory #430

batzkass opened this issue Feb 10, 2021 · 22 comments

Comments

@batzkass
Copy link

Hi Davide,
I hope you're going well.

I'm facing a technical issue these days with remi, due to one of its limitation. Today I worked this around with subclasses and method replacement in remi, but I think this could be implemented as its a generic feature.

Here it is: I need to tell remi that the "website" root isn't at xx.xx.xx.xx:XXXX/, but rather at xx.xx.xx.xx:XXXX/my_defined_root_string. The solution in remi is basically to concatenate this my_defined_root_string variable before all the hard links that are sent to the client. I know remi is far from being a CMS, but I can do a parallel with wordpress, which has a variable that stores this full server address to be able to rightly write all hard links.

Without this functionality, its not possible to use it in a serious server environment with multiple users and multiple remi instances. I explain : in a server, you must enter by the standard http or https port, and this must be shared for all remi instances, plus maybe other web services. So what I did is to install apache listening on port 80 from the internet, and multiple instances of remi for multiple apps/users listening on high-levels ports (8000,8001, etc...) from localhost. Imagine my server has a DNS name myserver.com. Apache plays the role of a proxy server: with a couple of rules, it takes the input queries on myserver.com:80/my_defined_root_string_numberX and regarding the address suffix (my_defined_root_string_numberX), redirects it to the right instance of remi on localhost:800X. This works great, even the websocket works, but I needed to tweak remi a bit to make it work: tweak all the "hard" links sent to the client with /my_defined_root_string_number1/.

It works well for me, but I think it would be helpful for remi to include this option.

Regards,
Francois

@dddomodossola
Copy link
Collaborator

Hello @batzkass ,
I'm pretty fine thank you. I hope you are well too.
Different users asked for the same thing and I know how it could be useful. This change could cause cascading problems especially for resources paths (a big devil fixed with hard work). Are you able to provide an initial implementation of this? Maybe we can create a separate branch with this functionality

@batzkass
Copy link
Author

I realize that my configuration is a bit "fancy" : the frontend apache acts as a proxy, and unpacks the incoming query address then removes the my_defined_root_string_numberX. This way, the resource paths issue is automatically solved and the only thing to do is to tweak the links inside the widgets that need it (and in js the server ws address to reach also) sent to the client.

But that's not a viable option for remi. At a first glance it doesn't seems difficult to me to adapt resource paths, can you please provide me some information on the main difficulties it could raise ? My understanding of the resource paths you made is the following: we want the webserver to have multiple resource dirs, unlike classical webservers that directly link the dirs tree to the client queries. To overcome that, you wrote a syntax with the : char, which helps the server to know in which resource directory it should look for the file. Don't you think that it would be as "simple" as parsing the address somewhere here and there to remove my_defined_root_string_numberX ?

@dddomodossola
Copy link
Collaborator

Your understanding of dirs is correct. It is required also to fix this regex https://github.com/dddomodossola/remi/blob/35b74690acf12adb2bf6a3f13c05695393451db6/remi/server.py#L646 . That regex is important to validate the path.

@batzkass
Copy link
Author

Ok so I'll have a try and keep you informed.

@batzkass
Copy link
Author

I started to work on it and I feel like it will work. I have one question however: the variable containing my_defined_root_string_numberX, lets call it query_root is currently handled by the App instance as an attribute. But I also need to access this attribute from gui.py to prefix the static urls, i.e in Image(...).
What's the better way to do that ?

@batzkass
Copy link
Author

Also, could you please tell me what is the purpose of this decorator on Image and Video ? I modified the __init__ and set_image methods to include query_root but should I also do something on those decorators ?

@dddomodossola
Copy link
Collaborator

I can't find an elegant way to transfer that variable query_root to gui.py. I will tell you if I will get an idea.
That decorator is used to allow set instance property for the widget. Instead of doing:

my_image.set_image(path) 

you can do also:

my_image.attr_src = path

So you cannot fix it, the user should do manually :

my_image.attr_src = query_root + path

I think it is not so elegant that the user passes query_root to Image and Video constructors. It should be done internally. Otherwise the user code loses maintainability. Consider if a user should change that value all around its code. I have not a better idea, it's just a consideration.

@batzkass
Copy link
Author

I can't find an elegant way to transfer that variable query_root to gui.py. I will tell you if I will get an idea.
That decorator is used to allow set instance property for the widget. Instead of doing:

my_image.set_image(path) 

you can do also:

my_image.attr_src = path

So you cannot fix it, the user should do manually :

my_image.attr_src = query_root + path

I think it is not so elegant that the user passes query_root to Image and Video constructors. It should be done internally. Otherwise the user code loses maintainability. Consider if a user should change that value all around its code. I have not a better idea, it's just a consideration.

My aim is to modify the decorator to handle this, so that it is transparent to the user. At this time I have a minimal working example, that's great.

@batzkass
Copy link
Author

Don't you think that def attr_src(self, value): self.attributes['src'] = query_root+str(value) will basically do the trick ? (assuming query_root is a global variable in gui.py, that's my temporary solution).
Note that for most users query_root=="", which will not cause any modification.

@dddomodossola
Copy link
Collaborator

It could work. Maybe the query_root name can be changed into url_root? Do you think could be better?

@batzkass
Copy link
Author

It could work. Maybe the query_root name can be changed into url_root? Do you think could be better?

Haha, I knew query_root was not ideal, just for tests before really finding the exact name. url_root is nice, I just wanted to see on other servers if there is a common name for that.

@batzkass
Copy link
Author

I forked remi and my implementation is here : https://github.com/batzkass/remi/tree/url_root
It comes with a documentation in README.md for remi/apache setup. Note that the implementation may lack some widgets at this time, I will need your help to do so.

@batzkass
Copy link
Author

In readme.md, the last "todo" line about style.css will be problematic. We should talk about that.

@batzkass
Copy link
Author

batzkass commented Feb 15, 2021

In readme.md, the last "todo" line about style.css will be problematic. We should talk about that.

Fixed, see readme.

@dddomodossola
Copy link
Collaborator

@batzkass I am unable to test/improve it in a short time. I'm involved in different projects. However I merged it in a separate branch as is. Thank you a lot for your effort and contribution.

@batzkass
Copy link
Author

@batzkass I am unable to test/improve it in a short time. I'm involved in different projects. However I merged it in a separate branch as is. Thank you a lot for your effort and contribution.

Sure I understand. I solved everything I think, so it should work nicely. Two items are still opened (see readme):

  • try https/wss : I will do it next week
  • change url_root access from gui.py, which will need you to think about.

@batzkass
Copy link
Author

batzkass commented Mar 15, 2021

Hi Davide,
Would it be possible for you to schedule a bit of time to integrate this feature inside remi ? As I wrote here, I made almost everything. What remains to you is to review code design and maybe a little of testing.
Thanks.

@dddomodossola
Copy link
Collaborator

Dear @batzkass ,

In this period I'm focused to some payd project, and on the C++ porting of remi. Furthermore I'm doing some researches about FPGA programming. I'm really busy. I merged your changes in a separate branch. I cannot test it actually and can't promise you to merge to master branch in a near future.
Can you please tell me if there is a necessity to you to merge to master branch in a short time? and why? If you really need it will try to do it in a near future.

Kind Regards

@batzkass
Copy link
Author

Nice projects ! I don't really need it in a short time, as I have setup my servers to download remi from my fork. It's rather that I can't afford to maintain my fork/branch regarding other modifications you may do to remi in parallel. So I just want to avoid this task to be forgotten, and reach a point that merging becomes very complicated.

@gbrault
Copy link
Contributor

gbrault commented Nov 12, 2021

Hello, I am interested by this feature, which branch of Remi should I take ?

@batzkass
Copy link
Author

Hi,
You can clone the url_root branch on my fork to get this feature : https://github.com/batzkass/remi/tree/url_root/remi
95% of the work is done, however I'm still waiting on Davide to make the final integration. Meanwhile, it's a bit dirty as you must set the (same) url_root at two places:

@gbrault
Copy link
Contributor

gbrault commented Nov 12, 2021

@batzkass : thanks!

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

No branches or pull requests

3 participants