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

Setup for Netlify deployments #20

Merged
merged 6 commits into from
Jul 23, 2018
Merged

Conversation

rbreslow
Copy link
Contributor

@rbreslow rbreslow commented Jul 20, 2018

Overview

Cursory Netlify TOML config file definition.

The repo was setup under the Azavea Ops Netlify user in the Shared-Azavea Ops LastPass folder.

  • Remove docker-compose.yml
  • Remove pip, docker, unzip, ntp, and python-security Ansible roles
  • Add azavea.nodejs and azavea.yarn Ansible roles
  • Update STRTA to execute in the context of the dev VM
  • Update Vagrantfile to use ansible_local
  • Remove Nginx and restructure project files since this will be deployed on Netlify

Resolves #11

Notes

The only "gotcha" I ran into was getting Netlify to install Yarn. Netlify looks for a yarn.lock file to determine if it should install Yarn. I had the project base directory set as ., and I had to change it to ./src, and then update the other paths to behave relative to ./src.

Netlify doesn't have shellcheck installed.

Also, I feel like Netlify's documentation leaves a lot in the air and is unfinished.

See: https://www.netlify.com/docs/build-gotchas/#yarn

Testing

See Netlify deploy log: https://app.netlify.com/sites/idb-osm-extraction-tool/deploys/5b5613713813f05b6d20e520

Visit: https://5b5613713813f05b6d20e520--idb-osm-extraction-tool.netlify.com

@rbreslow rbreslow self-assigned this Jul 20, 2018
@rbreslow rbreslow force-pushed the feature/jrb/setup-netlify branch 2 times, most recently from d9b34f1 to 93264b2 Compare July 23, 2018 15:22
* Remove docker-compose.yml
* Remove pip, docker, unzip, ntp, and python-security Ansible roles
* Add azavea.nodejs and azavea.yarn Ansible roles
* Update STRTA to execute in the context of the dev VM
* Update Vagrantfile to use ansible_local and rsync for livereload
* Remove Nginx and restructure project files since this will be deployed on Netlify
@kellyi
Copy link
Contributor

kellyi commented Jul 23, 2018

Going to set this up locally to check the impact of the dev environment changes.

@@ -1,14 +1,5 @@
- src: azavea.pip
version: 1.0.0
- src: azavea.nodejs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, how frequently will this nodejs version change major versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh -- I meant the azavea.nodejs role but it may be that there's no difference. Will the way this is set up also jump between LTS and non-LTS versions?

Copy link
Contributor Author

@rbreslow rbreslow Jul 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We feed the NodeJS version in deployment/ansible/group_vars/all and the azavea.nodejs role installs whatever we specify. It's like a smart robot that can just pull whatever NodeJS version we ask of it.

Versioning for the role itself is just stuff like bug fixes and updating where to look for packages.

See: https://github.com/azavea/ansible-nodejs/blob/develop/CHANGELOG.md

@kellyi
Copy link
Contributor

kellyi commented Jul 23, 2018

I'm seeing this error locally when trying to run ./scripts/cibuild:

Child html-webpack-plugin for "index.html":
     1 asset
    Entrypoint undefined = index.html
       [0] ./node_modules/html-webpack-plugin/lib/loader.js!./template.html 621 bytes {0} [built] [failed] [1 error]                                                                                                                  

    ERROR in ./template.html (./node_modules/html-webpack-plugin/lib/loader.js!./template.html)
    Module build failed: TypeError: options.name is not a function
        at Object.start (/vagrant/src/node_modules/html-minifier/src/htmlminifier.js:962:21)
        at handleStartTag (/vagrant/src/node_modules/html-minifier/src/htmlparser.js:376:15)
        at new HTMLParser (/vagrant/src/node_modules/html-minifier/src/htmlparser.js:181:11)
        at minify (/vagrant/src/node_modules/html-minifier/src/htmlminifier.js:951:3)
        at Object.exports.minify (/vagrant/src/node_modules/html-minifier/src/htmlminifier.js:1306:16)
        at Object.module.exports (/vagrant/src/node_modules/html-loader/index.js:128:26)
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Checking into it.

@kellyi
Copy link
Contributor

kellyi commented Jul 23, 2018

Another q mostly for my curiosity:

Is there a deployment-related reason to prefer keeping this project in Vagrant + Ansible rather than dropping the VM altogether and building it directly using npm? (Assuming, of course, that developers could use nvm to keep local Node.js versions in sync).

It seems like that's what Netlify might do by default, according to a specified Node.js version.

@kellyi
Copy link
Contributor

kellyi commented Jul 23, 2018

cibuild error is also happening on develop. Appears that a dependency of html-webpack-plugin may have gotten updated surreptitiously, so I'm trying to figure what is the easiest way to fix it.

@rbreslow rbreslow requested a review from kellyi July 23, 2018 17:01
@kellyi
Copy link
Contributor

kellyi commented Jul 23, 2018

I was able to get the cibuild step to complete & generate output by applying this patch to the current branch:

From 55be5401a73db4796148d9751bb31201e019e973 Mon Sep 17 00:00:00 2001
From: Kelly Innes <kinnes@azavea.com>
Date: Mon, 23 Jul 2018 13:01:00 -0400
Subject: [PATCH] Adjust html-loader option & output dir path

---
 src/webpack.common.config.js | 4 +++-
 src/webpack.prod.config.js   | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/webpack.common.config.js b/src/webpack.common.config.js
index e65ff1a..a024c16 100644
--- a/src/webpack.common.config.js
+++ b/src/webpack.common.config.js
@@ -80,7 +80,9 @@ module.exports = {
             },
             {
                 test: /\.(html)$/,
-                loader: 'html-loader?name=[name].[ext]',
+                use: [
+                    'html-loader',
+                ],
             },
             {
                 test: require.resolve('leaflet'),
diff --git a/src/webpack.prod.config.js b/src/webpack.prod.config.js
index 6431c3a..85bf143 100644
--- a/src/webpack.prod.config.js
+++ b/src/webpack.prod.config.js
@@ -3,7 +3,7 @@ var path = require('path');
 var config = require('./webpack.common.config');
 
 config.mode = 'production';
-config.output.path = path.join(__dirname, '..', '..', 'foo.bar');
+config.output.path = path.join(__dirname, '../dist');
 
 config.plugins.push(
     new webpack.DefinePlugin({
-- 
2.18.0

However, I noticed the files in the Vagrant VM aren't syncing with edits I make on host. I made the changes above in my host editor first but they weren't reflected in the VM, so I had to also make the edits in the VM to get things to work.

I'm not sure the dir path used above is correct, but it doesn't seems like these files exactly match either:

vagrant@vagrant:/vagrant/dist$ ls -lah
total 908K
drwxr-xr-x 2 vagrant vagrant 4.0K Jul 23 17:09 .
drwxr-xr-x 9 vagrant vagrant 4.0K Jul 23 17:01 ..
-rw-rw-r-- 1 vagrant vagrant 275K Jul 23 17:09 0.bundle.bb80302922c6a3462466.js
-rw-rw-r-- 1 vagrant vagrant 5.0K Jul 23 17:09 1.bundle.bb80302922c6a3462466.js
-rw-rw-r-- 1 vagrant vagrant 608K Jul 23 17:09 app.bundle.bb80302922c6a3462466.js
-rw-rw-r-- 1 vagrant vagrant  577 Jul 23 17:09 index.html
-rw-rw-r-- 1 vagrant vagrant    1 Jul 23 17:09 version.txt
vagrant@vagrant:/vagrant/dist$ exit
logout
Connection to 127.0.0.1 closed.
kinnes@allegheny:~/dvlp/idb-osm-extraction-tool     (ki/adjust-html-loader-options)$ ls -lah dist/
total 1776
drwxr-xr-x   7 kinnes  staff   224B Jul 23 12:58 .
drwxr-xr-x  16 kinnes  staff   512B Jul 23 13:10 ..
-rw-r--r--   1 kinnes  staff   274K Jul 23 12:58 0.bundle.c9c197e6e1e46cf6a997.js
-rw-r--r--   1 kinnes  staff   3.4K Jul 23 12:58 1.bundle.c9c197e6e1e46cf6a997.js
-rw-r--r--   1 kinnes  staff   599K Jul 23 12:58 app.bundle.c9c197e6e1e46cf6a997.js
-rw-r--r--   1 kinnes  staff   577B Jul 23 12:58 index.html
-rw-r--r--   1 kinnes  staff     8B Jul 23 12:58 version.txt
kinnes@allegheny:~/dvlp/idb-osm-extraction-tool     (ki/adjust-html-loader-options)$

@rbreslow
Copy link
Contributor Author

Is there a deployment-related reason to prefer keeping this project in Vagrant + Ansible rather than dropping the VM altogether and building it directly using npm? (Assuming, of course, that developers could use nvm to keep local Node.js versions in sync).

There isn't a deployment-related reason.

But, for example, I don't have NodeJS installed on my laptop. I do all my work in the context of a development VM. This way, I know I will always be using the correct version of a dependency, I won't need to worry about managing dependencies, and I know that any reason the build fails won't be due to the environment. My laptop stays squeaky clean.

It seems like that's what Netlify might do by default, according to a specified Node.js version.

I'm not sure what they're doing behind the scenes to install NodeJS and Yarn, but you can set the NODE_VERSION and YARN_VERSION environment variables to have the build environment pinned to a specific version.

I am updating the PR with more information on this, and pushing another commit up.

See: https://www.netlify.com/docs/build-settings/

cibuild error is also happening on develop. Appears that a dependency of html-webpack-plugin may have gotten updated surreptitiously, so I'm trying to figure what is the easiest way to fix it.

Yeah, I tested develop as well. I'm not that familiar with the webpack plugin ecosystem.

@rbreslow
Copy link
Contributor Author

rbreslow commented Jul 23, 2018

I noticed the files in the Vagrant VM aren't syncing with edits I make on host.

I modified Vagrantfile to use rsync to avoid any problems with live reload and sendfile.

$ vagrant rsync-auto

@kellyi
Copy link
Contributor

kellyi commented Jul 23, 2018

I don't have NodeJS installed on my laptop. I do all my work in the context of a development VM. This way, I know I will always be using the correct version of a dependency, I won't need to worry about managing dependencies, and I know that any reason the build fails won't be due to the environment

Sounds good to me!

$ vagrant rsync-auto

Should we add this to the setup script to run after the VM has been created?

@kellyi
Copy link
Contributor

kellyi commented Jul 23, 2018

Looks like Netlify deploys are working now! https://deploy-preview-20--idb-osm-extraction-tool.netlify.com/

I can't get the rsync change to work locally -- I tried out making an edit to the source for one of the components while webpack-dev-server was running and it didn't seem to live-reload. Removing the rsync-related block here -- https://github.com/azavea/idb-osm-extraction-tool/pull/20/files#diff-23b6f443c01ea2efcb4f36eedfea9089R12 -- and reprovisioning brought live-reloading during development back though.

Could we drop the rsync-related changes from the Vagrant file here and defer them to future work? Seems like the Netlify part of this is working really well!

@rbreslow
Copy link
Contributor Author

rbreslow commented Jul 23, 2018

scripts/setup is used to set up a project in an initial state. This is typically run after an initial clone, or, to reset the project back to its initial state.

Running rsync-auto is something we'd want to do every time we use the VM. vagrant up --provision will trigger an initial rsync to occur. Also, the command runs in the background to watch directories, so if we added it to scripts/setup, setup would never exit.

Could we drop the rsync-related changes from the Vagrant file here and defer them to future work?

Yes. That was preemptive on my part, because I saw that we used webpack-dev-server, and we've ran into problems before that were resolved with rsync.

@rbreslow rbreslow changed the title WIP: Setup netlify for deployments Setup for Netlify deployments Jul 23, 2018
Copy link
Contributor

@kellyi kellyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Netlify build is working and cibuild and server both work as expected locally!

Copy link

@hectcastro hectcastro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to get the STRTAs to work locally and my local code changes were reflected inside the VM. 👍

scripts/lint Outdated
-f docker-compose.yml \
run --rm --entrypoint ./node_modules/.bin/eslint \
app js/src/ --ext .js --ext .jsx
./node_modules/.bin/eslint \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to trap this as a package.json script called lint so that we can just use yarn run lint.

exit 1
fi
}
#yarn run test

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make an issue to add tests. In the past, we've commented out test execution in a STRTA and it went on to silently pass without executing tests for months.

@rbreslow rbreslow mentioned this pull request Jul 23, 2018
@rbreslow
Copy link
Contributor Author

I just found out that Netlify doesn't support shellcheck. 😬

@rbreslow rbreslow merged commit 45df5cf into develop Jul 23, 2018
@rbreslow rbreslow deleted the feature/jrb/setup-netlify branch July 23, 2018 20:22
@hectcastro
Copy link

Are you seeing anything when you navigate to https://idb-osm-extraction-tool.netlify.com/? That is the reported URL, but I'm getting a page not found error.

@rbreslow
Copy link
Contributor Author

That is the URL for master. The develop URL is https://develop--idb-osm-extraction-tool.netlify.com/.

See: https://www.netlify.com/docs/continuous-deployment/#branches-deploys

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.

3 participants