-
Notifications
You must be signed in to change notification settings - Fork 26
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
Update nodejs packages #111
Update nodejs packages #111
Conversation
ac06c6c
to
12725d7
Compare
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
|
6675635
to
905709d
Compare
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. |
905709d
to
50fad7d
Compare
I'm not sure what's going on because the build step runs without issue:
But I'm still getting the |
@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 |
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.
Alright, my build process must have been interrupted originally because it builds great with no issue adding --no-cache
. Nice work, @augusto-herrmann!!
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. |
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 |
50fad7d
to
33bd99f
Compare
@todrobbins would this be enough (297a67c)? 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?) |
297a67c
to
bf8432b
Compare
bf8432b
to
4e6f9c9
Compare
@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
docker build --rm -t publicbodies .
then run the container with
docker run --rm --volume="$PWD:/home/node/portal" -p 3000:3000 -it publicbodies node index.js
The site should appear on localhost:3000 .