chore: deploy docs|code .angularjs.org to Firebase via Travis by Narretz · Pull Request #16093 · angular/angular.js

Narretz · 2017-07-11T12:26:51Z

code.angularjs.org and docs.angularjs.org are two separate Firebase projects
both are automatically deployed via Travis config
Travis is split up into 2 build stages: first, all tests are run, and if they pass, the deploy
stage runs a single job with both deployments (actual deployment depends on the state of the commit)
docs. is deployed directly to Firebase hosting
code. is uploaded to Firebase Google Cloud Storage and uses Firebase hosting rewrites to acces the
files
jenkins builds still push the code builds to the code.angularjs.org Github repository

Further notes:

the caching for the docs app is currently the default (3600)
currently the secure firebase key is derived from my personal email account login. Should we create a key from a more internal google account instead?

Narretz · 2017-07-11T19:44:45Z

scripts/code.angularjs.org-firebase/functions/index.js

+    }).then(() => {
+      return response.status(200)
+        .set({
+          'Cache-Control': 'public, max-age=300, s-maxage=600'


We can tweak the cache values later: s-maxage is the value for the Firebase CDN, max-age for the browser

Perhaps we can move these values to configuration constants somewhere?

petebacondarwin

This is looking great @Narretz.
I have a left a few minor comments.

@IgorMinar - shall we generate secrets from a central account?

petebacondarwin · 2017-07-12T05:22:59Z

.gitignore

 *.swp
 angular.js.tmproj
 /node_modules/
+/scripts/code.angularjs.org-firebase/functions/node_modules/


This seems a bit hard coded.

If we just change the previous line to node_modules, without the slashes I think we would also capture this additional folder further down. Are there some node_modules folders in the site that we do want to capture?

An alternative would be to put a .gitignore at the /scripts/code.angularjs.org-firebase/functions folder level.

I don't think there's a node_modules that is checked in. Will change

petebacondarwin · 2017-07-12T05:29:37Z

.travis.yml

+    on_start: always
+jobs:
+  include:
+    - stage: deploy


So you removed all the indentation from lists in all the previous lines but then from here on down the indentation is there. I think we should be consistent. My understanding is that it is conventional to have indentation. Was there a reason for removing it?

This happened when I used the command line travis interface to add a value to the travis.yml. I will restore the original indentation

petebacondarwin · 2017-07-12T05:36:18Z

firebase.json

+      {
+        "source": "/",
+        "destination": "/index-production.html",
+        "type": 301


So you could not get the rewrites to work for / and /index.html?
This doesn't feel that nice, since people going to / will actually see the browser url change to /index-production.html.
If we can't get rewriting to work, perhaps we should just rename the file before deploying to docs?

Yes, the rewrites don't work because static matches take precedence over them. But what works (thanks to @gkalpak for the idea) is not to upload index.html, and rewrite / and index.html to index-production.html

petebacondarwin · 2017-07-12T05:37:23Z

readme.firebase.docs.md

+The docs are deployed to Google Firebase hosting via Travis deployment config, which expects
+firebase.json and .firebaserc in the repository root.
+
+See travis.yml for the complete deployment config.


It might be worth linking to scripts/code.angularjs.org-firebase/readme.firebase.code.md (the different code.angularjs.org deployment) in this file too.

petebacondarwin · 2017-07-12T05:44:24Z

scripts/code.angularjs.org-firebase/firebase.json

+      {
+        "source": "/:version/docs",
+        "destination": "/:version/docs/index.html",
+        "type": 301


I am less concerned about this redirect but it is a bit sad that Firebase doesn't support it via rewrites :-(.
Would a redirect from /:version/docs to /:version/docs/ work?

For some reason, redirect to /:version/docs/ results in an infinite redirect loop in the browser 🤔

petebacondarwin · 2017-07-12T05:52:13Z

scripts/code.angularjs.org-firebase/functions/index.js

+    }).then(() => {
+      return response.status(200)
+        .set({
+          'Cache-Control': 'public, max-age=300, s-maxage=600'


Perhaps we can move these values to configuration constants somewhere?

petebacondarwin · 2017-07-12T05:53:01Z

scripts/code.angularjs.org-firebase/functions/index.js

+function sendStoredFile(request, response) {
+  let filePathSegments = request.path.split('/').filter((segment) => {
+    return segment !== '';
+  });


This is to split off // from the path?

Would path.normalize() also work?

It's used to remove empty entries from the array coming from leading or trailing slashes in the path

petebacondarwin · 2017-07-12T05:58:03Z

scripts/code.angularjs.org-firebase/functions/index.js

+    return response.status(error.code).send(message);
+  });
+
+  function downloadAndSend() {


Could you pass the downloadPath and fileName to this function as parameters?
Accessing them from the closure makes the code harder to follow, since we have to check what might have changed these value throughout the code.

petebacondarwin · 2017-07-12T05:59:46Z

scripts/code.angularjs.org-firebase/functions/index.js

+    let message = 'General error';
+    if (error.code === 404) {
+      if (fileName.split('.').length === 1) {
+        message = 'Directory listing is not supported';


This is a shame. Could we look into supporting this in a future PR?

Could we host directly from Google storage (something like this)?

Probably. But I'm not convinced it is worth the effort to basically split the config in two. At the moment, direct access to gcs is only needed to upload the files, everything else goes through firebase

petebacondarwin · 2017-07-12T06:00:38Z

scripts/code.angularjs.org-firebase/readme.firebase.code.md

+in functions/index.js that serves the docs from the Firebase Google Cloud Storage bucket.
+
+The deployment to the Google Cloud Storage bucket happens automatically via Travis. See the travis.yml
+file in the repository root.


Could be worth a link to the other doc here, just in case people come across this file when they are looking for the other firebase configuration.

gkalpak · 2017-07-12T10:43:10Z

firebase.json

+    "public": "build/docs",
+    "ignore": [
+      "/index-debug.html",
+      "/index-jquery.html"


Can't we ignore /index.html as well? What is it used for?

Nothing, but Firebase logs a warning if there's no index.html. Not a problem, though.
Actually it works better without the index.html, see below.

gkalpak · 2017-07-12T11:01:12Z

scripts/code.angularjs.org-firebase/functions/index.js

+function sendStoredFile(request, response) {
+  let filePathSegments = request.path.split('/').filter((segment) => {
+    return segment !== '';
+  });


Would path.normalize() also work?

gkalpak · 2017-07-12T11:07:10Z

scripts/code.angularjs.org-firebase/functions/index.js

+    let message = 'General error';
+    if (error.code === 404) {
+      if (fileName.split('.').length === 1) {
+        message = 'Directory listing is not supported';


Could we host directly from Google storage (something like this)?

Narretz

I've addressed all your comments - only the redirect in code seems unfixable and directory listing / gcs static hosting can be handled later

Narretz · 2017-07-12T15:29:26Z

.gitignore

 *.swp
 angular.js.tmproj
 /node_modules/
+/scripts/code.angularjs.org-firebase/functions/node_modules/


I don't think there's a node_modules that is checked in. Will change

Narretz · 2017-07-12T15:30:15Z

.travis.yml

+    on_start: always
+jobs:
+  include:
+    - stage: deploy


This happened when I used the command line travis interface to add a value to the travis.yml. I will restore the original indentation

Narretz · 2017-07-12T15:31:16Z

firebase.json

+    "public": "build/docs",
+    "ignore": [
+      "/index-debug.html",
+      "/index-jquery.html"


Nothing, but Firebase logs a warning if there's no index.html. Not a problem, though.
Actually it works better without the index.html, see below.

Narretz · 2017-07-12T15:32:29Z

firebase.json

+      {
+        "source": "/",
+        "destination": "/index-production.html",
+        "type": 301


Yes, the rewrites don't work because static matches take precedence over them. But what works (thanks to @gkalpak for the idea) is not to upload index.html, and rewrite / and index.html to index-production.html

Narretz · 2017-07-12T15:36:04Z

scripts/code.angularjs.org-firebase/functions/index.js

+function sendStoredFile(request, response) {
+  let filePathSegments = request.path.split('/').filter((segment) => {
+    return segment !== '';
+  });


It's used to remove empty entries from the array coming from leading or trailing slashes in the path

Narretz · 2017-07-12T15:40:33Z

scripts/code.angularjs.org-firebase/functions/index.js

+    let message = 'General error';
+    if (error.code === 404) {
+      if (fileName.split('.').length === 1) {
+        message = 'Directory listing is not supported';


Probably. But I'm not convinced it is worth the effort to basically split the config in two. At the moment, direct access to gcs is only needed to upload the files, everything else goes through firebase

Narretz · 2017-07-12T16:16:39Z

scripts/code.angularjs.org-firebase/firebase.json

+      {
+        "source": "/:version/docs",
+        "destination": "/:version/docs/index.html",
+        "type": 301


For some reason, redirect to /:version/docs/ results in an infinite redirect loop in the browser 🤔

Narretz · 2017-07-12T16:57:42Z

scripts/code.angularjs.org-firebase/functions/index.js

+const gcs = require('@google-cloud/storage')();
+const path = require('path');
+
+const gcsBucket = 'code-angularjs-org-338b8.appspot.com';


There's an environment variable that we can use: process.env.GCLOUD_PROJECT

- code.angularjs.org and docs.angularjs.org are two separate Firebase projects - both are automatically deployed via Travis config - Travis is split up into 2 build stages: first, all tests are run, and if they pass, the deploy stage runs a single job with both deployments (actual deployment depends on the state of the commit) - docs. is deployed directly to Firebase hosting - code. is uploaded to Firebase Google Cloud Storage and uses Firebase hosting rewrites to acces the files - jenkins builds still push the code builds to the code.angularjs.org Github repository Closes angular#9674

petebacondarwin · 2017-07-12T21:16:19Z

@IgorMinar - shall we generate secrets from a central account?

He suggests that we go with the secrets that we have for now as we don't have a general solution to this yet.

- code.angularjs.org and docs.angularjs.org are two separate Firebase projects - both are automatically deployed via Travis config - Travis is split up into 2 build stages: first, all tests are run, and if they pass, the deploy stage runs a single job with both deployments (actual deployment depends on the state of the commit) - docs. is deployed directly to Firebase hosting - code. is uploaded to Firebase Google Cloud Storage and uses Firebase hosting rewrites to acces the files - jenkins builds still push the code builds to the code.angularjs.org Github repository Closes angular#9674 Closes angular#16093

- code.angularjs.org and docs.angularjs.org are two separate Firebase projects - both are automatically deployed via Travis config - Travis is split up into 2 build stages: first, all tests are run, and if they pass, the deploy stage runs a single job with both deployments (actual deployment depends on the state of the commit) - docs. is deployed directly to Firebase hosting - code. is uploaded to Firebase Google Cloud Storage and uses Firebase hosting rewrites to acces the files - jenkins builds still push the code builds to the code.angularjs.org Github repository Closes #9674 Closes #16093

Narretz added frequency: high type: chore type: infrastructure labels Jul 11, 2017

Narretz added this to the 1.6.6 milestone Jul 11, 2017

Narretz requested review from gkalpak and petebacondarwin July 11, 2017 12:26

googlebot added the cla: yes label Jul 11, 2017

Narretz added the needs: review label Jul 11, 2017

Narretz commented Jul 11, 2017

View reviewed changes

petebacondarwin approved these changes Jul 12, 2017

View reviewed changes

gkalpak reviewed Jul 12, 2017

View reviewed changes

Narretz commented Jul 12, 2017

View reviewed changes

Narretz force-pushed the chore-firebase branch from 24892ef to 34b612a Compare July 12, 2017 17:03

Narretz added 2 commits July 12, 2017 19:13

fixup! chore: deploy docs|code .angularjs.org to Firebase via Travis

7cda96a

Narretz force-pushed the chore-firebase branch from 34b612a to 7cda96a Compare July 12, 2017 17:13

petebacondarwin approved these changes Jul 12, 2017

View reviewed changes

Narretz merged commit 828a275 into angular:master Jul 13, 2017

Narretz mentioned this pull request Jul 13, 2017

BEFORE WE FORK 1.6.x FROM master #15417

Closed

Comments

Conversation

Narretz commented Jul 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Narretz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Jul 12, 2017

Uh oh!

Reviewers

Assignees

Labels

Narretz commented Jul 11, 2017 •

edited

Loading