-
-
Notifications
You must be signed in to change notification settings - Fork 403
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
C9 #200
base: master
Are you sure you want to change the base?
C9 #200
Conversation
Hello Dirk, excuse me for the late reply. I'm travelling to Russia (Moscow) for working reasons. I will look at this in about a week. Thank you so much for contributinng in Remi! |
self._log.debug('ws ending websocket service') | ||
break | ||
try: | ||
clients[k].websockets.remove(self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @dirkcgrunwald !
Looking at changes you made, I suppose that beside others, these 8 grouped lines (from 150 to 158) are the necessary part to be C9 compatible. Others are refinements. Is this correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 2-3 others parts that are needed.
In if 'C9_HOSTNAME' in os.environ:
-
key = data.decode().split('sec-websocket-key: ')[1].split('\r\n')[0]
-
else:
-
key = data.decode().split('Sec-WebSocket-Key: ')[1].split('\r\n')[0]
it appears that the C9 proxy makes the response headers in lower case. It's also possible to use a try...except to try the upper case, fail and then revert to lower case.
In addition, in the default setup, if you use '0.0.0.0' as the host IP, remi changes this to 127.0.0.1, which isn't proxied. The user doesn't know the IP address of their server because of the proxy, so they need to leave '0.0.0.0' as an option. I had guarded those changes with checks for the C9 environment.
This may all be moot since C9 is being changed to AWS Cloud and I think those don't use the proxy.
@dirkcgrunwald Thank you so much for your contribution! Other people requested this feature previously and I was unable to fix. I merged it in a specific branch "dirkcgrunwald-c9", to make it available to other users. I will remove some specific C9 vars and separate the SVG related commits, than I will merge it to master (when I will have some time available). |
Cloud9 is a web IDE environment that uses a proxied connection that causes the base REMI library to not work correctly. It has a couple of other odd quirks. This PR fixes things such that people can use REMI in c9 - we're planning on this for some of our intro CS classes. There are probably better solutions and I'd be happy to modify things to such a solution (e.g. I'm using C9 environment variables to conditionalize specific actions, and it may make sense to use some notion of different "operating environments" since my proxy-aware code is C9 specific).