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

Added kaazing-gateway 5.0.1.46 release and Removed old Kaazing versio… #1445

Closed
wants to merge 2 commits into from
Closed

Conversation

dpwspoon
Copy link
Contributor

…n in favor of new version. Support for KWIC added via configuration.

@yosifkit
Copy link
Member

Sorry for the delay, I've been traveling the last week.

Changeset: kaazing/gateway.docker@ab90b20...d9630f7
That looks like a lot more than a version bump. I'll try to dive into it in the morning to try and understand the additions (or feel free to jump in and explain).

From my initial pass, I would highlight https://github.com/docker-library/official-images#consistency in the change from CMD to ENTRYPOINT.

@dpwspoon
Copy link
Contributor Author

Thanks @yosifkit, apologies for the misleading PR title, things inadvertently got added together. This updated the version and added support for a different type of configuration at runtime. The updated scripts work is generally done to make configuration of the gateway easier when wanting to use it for KWIC; that's a light weight VPN like solution that runs ontop of websockets.

I believe originally having the CMD vs ENTRYPOINT may always have been an error as the gateway.start script can take arguments such as --config. I'll make the change using and entrypoint with a CMD and fallout to bash if the command is not recognized. I'll ping you when that is done (heading home now and might not be tonight)

@yosifkit
Copy link
Member

Is there a reason for overwriting the gateway.start that comes with Kaazing? Couldn't this new gateway.start just be a new script that does an exec to the original gateway.start?

There appears to be quite a few variables defined at the top, but many seem to be unused. Are they supposed to be environment defaults that the server will use? Some might be better defined in the Dockerfile as ENV.

Also, the case line for ambassador-server is missing a shift. 😉

docker-library/docs#491 is the docs PR to go with this?

@dpwspoon
Copy link
Contributor Author

dpwspoon commented Mar 8, 2016

Thanks for the feedback, closing this for now as the script might get picked up by the main project. I'll open another one shortly to update the version

@dpwspoon dpwspoon closed this Mar 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rate limit · GitHub

Access has been restricted

You have triggered a rate limit.

Please wait a few minutes before you try again;
in some cases this may take up to an hour.

3 participants