ThauEx / ffrk-proxy

Proxy for Final Fantasy Record Keeper written in node.js
GNU Lesser General Public License v3.0
39 stars 21 forks source link

fix docker build #143

Closed jarrettone closed 5 years ago

jarrettone commented 6 years ago

Using git+ssh:// in package{,-lock}.json is just not suitable for deploying ffrk-proxy in a container. By simply switching to git+https:// the container then builds and runs perfectly, even on a Raspberry Pi running Docker! 😎

ThauEx commented 6 years ago

Thanks for your PR, but have you seen this?
https://github.com/jdel/docker-ffrk-proxy

jarrettone commented 6 years ago

@ThauEx I have! Building that container is actually what lead to me having the issues this PR fixes. Unfortunately, just using the container @jdel uploaded to Docker Hub won't work for me as my Docker environment runs on a Raspberry Pi (ARM) and that container was built on x86_64.

Even with these changes, I still end up needing the above referenced PR against @jdel's Dockerfile to add git to the container so that git+https works as a source for npm. If there's a way to avoid git sources in your package.json and package-lock.json, I'd be happy to look into it, including testing in Docker containers on both x86_64 and ARM, though the testing on x8664 may take me quite a bit longer as I currently don't have Docker on that architecture anywhere that isn't strictly for work purposes_...

ThauEx commented 6 years ago

Thanks for the explanation, but would you be so kind and add a Dockerfile? THen we would have a complete "solution" for that.

jarrettone commented 6 years ago

There's already a Dockerfile in jdel/docker-ffrk-proxy. I could pull the Docker bits from that repo into this one to make this a truly complete solution, but none of that has a license attached so @jdel would need to either add a license compatible with GPLv3 or give explicit permission to add that here and it would become GPLv3, here.

jarrettone commented 6 years ago

@ThauEx So far, I'm not getting any response from @jdel on https://github.com/jdel/docker-ffrk-proxy/pull/2, so I'm wondering if you'd be comfortable testing future changes to ffrk-proxy in a Docker container on an on-going basis. If you would, I'll commit to putting together a Dockerfile and supporting bits, like supplemental scripts, docs, etc., and submit a PR here for the same so that we have a full Docker-ized solution for ffrk-proxy both for ongoing dev and use.

Thoughts?

@jdel may still get back to us, so I'll definitely give a few days as I'd rather not ~stomp all over~ rewrite his work if I don't have to. I'd rather have his permission and pull his work into here, mostly as-is.

ThauEx commented 5 years ago

Hard to say, I'm not playing the game for a while now and just trying to fix issues. For most people using this, docker is not that important.
But docker support is still a nice to have. Is there much to maintain?

jarrettone commented 5 years ago

For most people using this, docker is not that important.

For this reason, I'd leave the Docker support separate. It's not a lot to maintain, but if most folks don't use Docker and just install Node locally, its probably not ideal to add the Docker-specific artifacts to your repo as they could become outdated and a source of tech debt.

Querying the community might give a different answer, though. Is there a relatively active forum of some sort that uses this project where this can be asked? I don't mind asking if you can point me in the right direction. I also don't mind stepping back and giving you time to ask if you'd prefer.

jarrettone commented 5 years ago

One thing I would like to add here, though, is a test to verify that NPM sources don't require SSH so that git+ssh:// URIs don't end up back in the package.json file. Any idea how to do this? My idea was to simply check for SSH in the values at ["dependencies"][*] in package.json and print a helpful error message about Docker needing git+https:// and simply switching from SSH to HTTPS will work fine for public GitHub repositories.