Closed augusto-herrmann closed 3 years ago
The code is working now with modern versions of packages and should be good enough to go.
Running on localhost gives some CSS differences. Looking at the console log, I can see the browser fails to load Google Fonts. I think this is due to some policy restrictions for localhost by the browser (using Firefox here). If it still happens on the host we can fix it later.
Anyone up for a code review? @rufuspollock
@augusto-herrmann looks great 👏 There's one conflict now - any way to resolve that so this is a clean merge? Then we can get this in ...
@augusto-herrmann I'm trying to test this build and get this error on running docker run
:
internal/modules/cjs/loader.js:818
throw err;
^
Error: Cannot find module 'express'
Require stack:
- /home/node/portal/index.js
at Function.Module._resolveFilename (internal/modules/cjs/loader.js:815:15)
at Function.Module._load (internal/modules/cjs/loader.js:667:27)
at Module.require (internal/modules/cjs/loader.js:887:19)
at require (internal/modules/cjs/helpers.js:74:18)
at Object.<anonymous> (/home/node/portal/index.js:5:15)
at Module._compile (internal/modules/cjs/loader.js:999:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
at Module.load (internal/modules/cjs/loader.js:863:32)
at Function.Module._load (internal/modules/cjs/loader.js:708:14)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12) {
code: 'MODULE_NOT_FOUND',
requireStack: [ '/home/node/portal/index.js' ]
}
Hi, @todrobbins . Did you follow the instructions above, in the original post? What exactly did you type in the terminal?
From your error message, not finding express, it looks like the npm install . step in the docker file did not execute correctly. Did you get any error messages during the docker build step?
Here is what I get in the docker run:
$ docker run --rm --volume="$PWD:/home/node/portal" -p 3000:3000 -it publicbodies node index.js
Express server listening on port 3000
Processing br.csv...
Processing ch.csv...
Processing de.csv...
Processing eu.csv...
Processing gb.csv...
Processing gr.csv...
Processing it.csv...
Processing np.csv...
Processing nz.csv...
Processing se.csv...
Processing us.csv...
Added 244 bodies from br.csv
Added 2785 bodies from ch.csv
Added 1003 bodies from de.csv
Added 129 bodies from eu.csv
Added 5362 bodies from gb.csv
Added 949 bodies from gr.csv
Added 11951 bodies from it.csv
Added 140 bodies from np.csv
Added 507 bodies from nz.csv
Added 652 bodies from se.csv
Added 458 bodies from us.csv
and the site gets served on http://localhost:3000.
PS: I've just rebased this PR so as to make it once again mergeable.
I'm not sure what's going on because the build step runs without issue:
➜ docker build --rm -t publicbodies .
[+] Building 0.1s (10/10) FINISHED
=> [internal] load build definition from Dockerfile 0.0s
=> => transferring dockerfile: 37B 0.0s
=> [internal] load .dockerignore 0.0s
=> => transferring context: 2B 0.0s
=> [internal] load metadata for docker.io/library/node:12 0.0s
=> [1/5] FROM docker.io/library/node:12 0.0s
=> [internal] load build context 0.0s
=> => transferring context: 34B 0.0s
=> CACHED [2/5] RUN mkdir -p /home/node/portal 0.0s
=> CACHED [3/5] COPY ./package.json /home/node/portal/package.json 0.0s
=> CACHED [4/5] WORKDIR /home/node/portal 0.0s
=> CACHED [5/5] RUN npm install . 0.0s
=> exporting to image 0.0s
=> => exporting layers 0.0s
=> => writing image sha256:7523a483e7b5cfd406c7e7ad5f1d0707824c78872f4bc223a6167d905a97779d 0.0s
=> => naming to docker.io/library/publicbodies
But I'm still getting the express
error after using your rebased PR.
@todrobbins that particular run of docker build isn't of much help in debugging because the essential steps are all already cached. Please try again with the --no-cache
option and watch out for any errors during the RUN npm install .
step of the Dockerfile.
Thanks, @todrobbins ! Do you know how we can deploy on the current infrastructure? Or if it would require a new VM because we're now using Docker containers?
@rufuspollock would you like to review this PR as well? Otherwise, I think we're good to merge.
Good question, I think the server setup would need to change since the site is currently deployed for Node.js via Heroku. Here's the current setup:
Actually, I'm amazed Heroku still supports Node.js < 1.0, or whatever we have in production, which is probably a decade old. But they do support Docker containers, of course, so I imagine it shouldn't be too difficult to migrate the production environment.
Also, I've created issue #115 so that we remember to automate deployment some time.
According to Heroku's documentation, we need to create a heroku.yml
file to deploy a custom built docker image.
@todrobbins would this be enough (297a67c6)? How can we test this deployment in Heroku?
@rufuspollock do you have the keys to deploy on Heroku? Can you give me access?
Additionally, now that we have automatically updated data (even if it's only Brazil for now), could we have some of that fronted face lift help you promised?
I'm no frontend expert, but if we could regularly, if not automatically, deploy the site code to Heroku I could at least try to do some improvements. For instance, the weekly updated Brazilian data has URLs for all of the images containing logos of public bodies. It would be awesome to display that on the UI. (BTW should those images be mirrored somewhere instead of served as hot links?)
@augusto-herrmann I can add you as a collaborator, just need your email address. Here's the current list of collaborators that we may want to review:
That's great, @todrobbins! I've signed up to Heroku and I've just sent you an email telling you the email address I've used there.
✅ Added
Fixes #110. I've updated the packages and code to get the site running on modern node.js, and using Docker.
To test the site, first build the container using
then run the container with
The site should appear on localhost:3000 .