Skip to content

Support deployments to Heroku #762

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

Merged
merged 3 commits into from
Jan 24, 2019

Conversation

francisli
Copy link
Contributor

Before your pull request is reviewed and merged, please ensure that:

  • there are no linting errors -- npm run lint
  • your code is in a uniquely-named feature branch and has been rebased on top of the latest master. If you're asked to make more changes, make sure you rebase onto master then too!
  • your pull request is descriptively named and links to an issue number, i.e. Fixes #123

Thank you!

We've been running our own instance of p5.js Web Editor on Heroku at very low cost and low administrative overhead for a while so I thought I'd share the changes back for consideration. I've provided a complete Heroku app.json configuration so that you can launch a complete environment, including provisioning free MongoLab Mongo DB and Mailgun add-ons, with one click. To try it for yourself, sign up for a free Heroku account and click this link: https://heroku.com/deploy?template=https://github.com/passion-for-coding/p5.js-web-editor/tree/heroku-deployment

(note that if this is approved for merge into mainline, then the template URL should change in the documentation to reflect that)

@catarak
Copy link
Member

catarak commented Nov 12, 2018

this is so awesome!

@francisli francisli mentioned this pull request Nov 13, 2018
@francisli
Copy link
Contributor Author

@catarak I was able to remove all the extra env var names by adding a new start:heroku command which maps how the companies name their vars to how they're expected in the code.

@francisli
Copy link
Contributor Author

Hold on, going to try moving the ENV var mapping into the Procfile, so we don't need an extra start command in the package.json file...

@francisli
Copy link
Contributor Author

Ok, I think things are in a good state now. The env var mapping is done in the Procfile, so no need for extra variable names in the code, nor any additional heroku specific scripts in package.json.

I've also added running the fetch-examples script to the postdeploy so that will also be automatically initialized.

@francisli
Copy link
Contributor Author

@catarak I've just resolved conflicts from master and re-squashed the branch. is there anything else you'd like me to do here?

@catarak
Copy link
Member

catarak commented Dec 11, 2018

Sorry for the delay on this. I think that your approach to the web editor is opening a lot of questions for me, in a good way. It's important to be open to other hosting besides the official editor hosting, but I'm not sure what that relationship is like. Should this repository be responsible for making sure other hosting guides are supported? Should it instead link out to outside guides? I don't know.

I want it to be clear that there's an endorsed way, the deployment that the official web editor is using, and also other ways. I could see there being an endorsed guide and a guide for smaller deployments. Heroku seems like a good target for the latter since it's free and easy to use. I'm concerned about two things:

  1. The Heroku configuration files getting out of date, since I will be deploying and testing deployment using Kubernetes/GKE.
  2. Configuration file cluttering and confusion. It could get confusing which config files are used for each type of deployment. Do other open source web apps have this problem? How do they approach it?

@francisli
Copy link
Contributor Author

@catarak well, I opened this PR because I thought the ease of deployment with Heroku offered pretty significant value to the potential audience of this project who might be educators, etc- I'm not aware of any other PAAS platform that has made it this simple to package and deploy with little-to-no administrative overhead.

I don't think there's any good answer or model example out there in the open source world, which I think just assumes everyone (reality: other software engineers like me) is knowledgeable enough to do whatever they want with the code.

However, I understand if you don't want to be responsible for it (even if it is pretty easy to test and maintain!), so if that's the case, feel free to close the PR and I'll just continue to maintain it in my own fork...

@catarak
Copy link
Member

catarak commented Dec 12, 2018

I'm supportive of empowering educators to be in charge of their tools, which can mean self hosting! I also think you're right about Heroku being a clear leader. And I think you're right about the maintenance being low! I think it also makes sense to keep these files with the web editor, so that it can be part of the Docker images.

I'm going to leave some comments to suggest changes to the documentation!

@catarak
Copy link
Member

catarak commented Dec 12, 2018

Also, have you ever heard of https://nanobox.io? I'm trying to move the web editor off AWS products, and it seems like it's a Heroku alternative that is cloud agnostic.

@francisli
Copy link
Contributor Author

Hmm, first time I've heard of nanobox! Very interesting... not sure I'd call it a "Heroku alternative", because you still need to manage all the details of the containers and the orchestration of supporting components themselves... for example, how would the MongoDB instance be managed- would you run it yourself in a Docker container vs in a separate fully managed service?

@catarak
Copy link
Member

catarak commented Dec 13, 2018

got it. i would absolutely suggest a separate fully managed mongodb service, like mlab.

@francisli
Copy link
Contributor Author

This should be good for a squash-merge per your latest change requests...!

@catarak
Copy link
Member

catarak commented Jan 24, 2019

thanks for updating this! i'm going to update this PR really quick so that the Node/npm versions are 10.15.0/6.4.1, but otherwise this is great.

@catarak catarak merged commit c91199f into processing:master Jan 24, 2019
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.

2 participants