[Wheelmap] service rewrite and tests #2486

PyvesB · 2018-12-08T21:54:59Z

Fixes issue #2469 and benefits from new functionnality in icedfrisby-nock!

Before we can deploy this, we have to coordinate on adding the API token to the production server, and optionally on Circle CI.

Looking forward to feedback! 👍

shields-ci · 2018-12-08T21:55:48Z

	Warnings
⚠️	This PR modified `package.json`, but not `package-lock.json` - Perhaps you need to run `npm install`?

	Messages
📖	✨ Thanks for your contribution to Shields, @PyvesB!
📖	Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS

…elmap

paulmelnikow

Thanks for picking this up!

services/wheelmap/wheelmap.service.js

paulmelnikow · 2018-12-08T22:47:11Z

services/wheelmap/wheelmap.tester.js

+// If no Wheelmap token is provided (e.g. local development), API
+// responses are intercepted and mocked. If a token is found, the
+// requests will be forwarded to the real Wheelmap service.
+const noToken = !serverSecrets.wheelmap_token


One thing I'm a little nervous about is tests failing silently when there's no token. What do you think about logging a warning when there's no token? If you do it here, it'll always print because the module is always loaded, though if you put it in a before() hook on each test, it'll only print when the tests are run.

I'm sorry, I wasn't clear about this.

If you hook it up like this:

function logTokenWarning() { ... } t.create(...) .before(logTokenWarning)

then it will only print when these service tests are actually run.

Oh yeah, what I did is indeed run regardless of the service, I'll fix that...

paulmelnikow · 2018-12-08T22:50:08Z

Regarding tokens, how would you feel about taking care of the two test environments and staging, and I'll take care of production?

The secrets get pushed from a json file on my dev machine which I can update on my own. @espadrine can fetch the latest version from the servers via git or ssh.

PyvesB · 2018-12-09T10:26:28Z

I've addressed the review comments and tried adding my API key to Heroku and to the Circle CI daily and on-commit projects, however, things don't seem to be wired up correctly. Are there any additional steps needed for the new configuration to be picked up?

paulmelnikow · 2018-12-09T11:26:41Z

Ahhh right, good question.

The change to secret.tpl.json is good, though it only affects Docker / self-hosting.

You'll need to update this line, which is what pulls the secret from the environment in the tests:

shields/.circleci/config.yml

Line 162 in b08a941

echo "{\"gh_token\":\"$GITHUB_TOKEN\"}" > private/secret.json

Heroku would need something equivalent put in place. That could be deferred if you like. Though I suppose the way to do it is adding a heroku-start script which writes private/secret.json only if it's not already checked in, so it doesn't conflict with self-hosting users who are doing that, and then runs node server. Procfile would need to be updated to run npm run heroku-start.

Added: Tests are passing locally using a real Wheelmap token.

PyvesB · 2018-12-09T13:03:58Z

The tests on Circle Ci now seem to be happily running with my token!

I've attempted an implementation for Heroku. However, the app is now failing to start, I'm unsure why as things seem fine when run locally.

paulmelnikow · 2018-12-09T13:46:19Z

The best way to debug is probably using the Heroku CLI and running heroku logs --app shields-staging-pr-2486.

This is what I'm seeing:

2018-12-09T13:08:18.423735+00:00 app[web.1]:
2018-12-09T13:08:18.423752+00:00 app[web.1]: > shields.io@0.0.0 heroku-start /app
2018-12-09T13:08:18.423753+00:00 app[web.1]: > node scripts/export-heroku-secrets-cli.js
2018-12-09T13:08:18.423755+00:00 app[web.1]:
2018-12-09T13:08:18.613175+00:00 app[web.1]: private/secret.json does not exist, creating.
2018-12-09T13:08:18.616829+00:00 app[web.1]: fs.js:121
2018-12-09T13:08:18.616832+00:00 app[web.1]: throw err;
2018-12-09T13:08:18.616834+00:00 app[web.1]: ^
2018-12-09T13:08:18.616835+00:00 app[web.1]:
2018-12-09T13:08:18.616837+00:00 app[web.1]: Error: ENOENT: no such file or directory, open '/app/private/secret.json'
2018-12-09T13:08:18.616839+00:00 app[web.1]: at Object.openSync (fs.js:450:3)
2018-12-09T13:08:18.616840+00:00 app[web.1]: at Object.writeFileSync (fs.js:1212:35)
2018-12-09T13:08:18.616842+00:00 app[web.1]: at Object.<anonymous> (/app/scripts/export-heroku-secrets-cli.js:14:6)
2018-12-09T13:08:18.616844+00:00 app[web.1]: at Module._compile (internal/modules/cjs/loader.js:723:30)
2018-12-09T13:08:18.616845+00:00 app[web.1]: at Object.Module._extensions..js (internal/modules/cjs/loader.js:734:10)
2018-12-09T13:08:18.616847+00:00 app[web.1]: at Module.load (internal/modules/cjs/loader.js:620:32)
2018-12-09T13:08:18.616848+00:00 app[web.1]: at tryModuleLoad (internal/modules/cjs/loader.js:560:12)
2018-12-09T13:08:18.616850+00:00 app[web.1]: at Function.Module._load (internal/modules/cjs/loader.js:552:3)
2018-12-09T13:08:18.616851+00:00 app[web.1]: at Function.Module.runMain (internal/modules/cjs/loader.js:776:12)
2018-12-09T13:08:18.616853+00:00 app[web.1]: at executeUserCode (internal/bootstrap/node.js:342:17)
2018-12-09T13:08:18.623278+00:00 app[web.1]: npm ERR! code ELIFECYCLE
2018-12-09T13:08:18.623787+00:00 app[web.1]: npm ERR! errno 1
2018-12-09T13:08:18.625725+00:00 app[web.1]: npm ERR! shields.io@0.0.0 heroku-start: `node scripts/export-heroku-secrets-cli.js`
2018-12-09T13:08:18.625963+00:00 app[web.1]: npm ERR! Exit status 1
2018-12-09T13:08:18.626250+00:00 app[web.1]: npm ERR!
2018-12-09T13:08:18.626444+00:00 app[web.1]: npm ERR! Failed at the shields.io@0.0.0 heroku-start script.
2018-12-09T13:08:18.626611+00:00 app[web.1]: npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
2018-12-09T13:08:18.647801+00:00 app[web.1]:
2018-12-09T13:08:18.648160+00:00 app[web.1]: npm ERR! A complete log of this run can be found in:
2018-12-09T13:08:18.648381+00:00 app[web.1]: npm ERR!     /app/.npm/_logs/2018-12-09T13_08_18_628Z-debug.log
2018-12-09T13:08:18.730908+00:00 heroku[web.1]: Process exited with status 1
2018-12-09T13:40:12.913176+00:00 heroku[web.1]: Process exited with status 1
2018-12-09T13:40:13.127701+00:00 heroku[web.1]: State changed from starting to crashed

PyvesB · 2018-12-09T15:12:36Z

That's now fixed, for some reason the logs in the Heroku UI were not as verbose as the ones you posted. I wanted to test this in the review app and set the token environement variable in shields-staging. I thought it would be propagated to newly deployed review apps as shields-staging is labelled as the "parent app". However, this does not seem to be the case. There ins't anything relevant in the pipeline settings either. Do you know if it's even possible to set that environment variable for all apps without having to specify it in app.json?

paulmelnikow · 2018-12-09T21:39:56Z

The difference is the logs in the UI are for the build. The ones from heroku logs are for runtime.

There’s a way to specify in app.json that an env var should inherit from the parent app. You don’t have to specify it explicitly. I’m on mobile so I don’t have the docs at my fingertips.

PyvesB · 2018-12-09T22:00:24Z

Thanks for pointing that out, I found the relevant part of the documentation after some additional digging. Unfortunately, the main section I was previously looking at did not mention anything about that. I'll give it a go.

PyvesB · 2018-12-09T22:25:53Z

🍾 Now we're talking:

shields-deployment · 2018-12-10T18:44:51Z

This pull request was merged to master branch. This change is now waiting for deployment, which will usually happen within a few days. Stay tuned by joining our #ops channel on Discord!

After deployment, changes are copied to gh-pages branch:

Rewrote the Wheelmap service

061cd42

PyvesB added bug Bugs in badges and the frontend service-badge New or updated service badge labels Dec 8, 2018

Removed unused constants

523b838

PyvesB changed the title ~~Rewrote the [Wheelmap] service and added tests~~ [Wheelmap] service rewrite and tests Dec 8, 2018

PyvesB added 3 commits December 8, 2018 21:59

Merge branch 'master' into wheelmap

78a4d3a

Used shorthands where possible

b0b552c

Merge branch 'wheelmap' of https://github.com/PyvesB/shields into whe…

207a1f2

…elmap

paulmelnikow reviewed Dec 8, 2018

View reviewed changes

PyvesB added 2 commits December 9, 2018 09:21

Addressed review comments

9e88ebb

Merge branch 'master' into wheelmap

bfa00bc

Updated Circle CI configuration

b92a5ae

PyvesB temporarily deployed to shields-staging-pr-2486 December 9, 2018 11:56 Inactive

Refactored test logging

84b0111

PyvesB temporarily deployed to shields-staging-pr-2486 December 9, 2018 12:05 Inactive

Added support for Heroku test secrets

2d71826

PyvesB temporarily deployed to shields-staging-pr-2486 December 9, 2018 12:36 Inactive

PyvesB force-pushed the wheelmap branch from db38772 to b0c8422 Compare December 9, 2018 12:50

Renamed file and switched to console.log

2b3e85e

PyvesB force-pushed the wheelmap branch from b0c8422 to 2b3e85e Compare December 9, 2018 12:58

PyvesB temporarily deployed to shields-staging-pr-2486 December 9, 2018 12:59 Inactive

Handled case where directory doesn't exist

4c8521f

PyvesB deployed to shields-staging-pr-2486 December 9, 2018 14:47 View deployment

PyvesB added 2 commits December 9, 2018 22:07

Added WHEELMAP_TOKEN to trigger inheritance from parent

e8cd872

Merge branch 'master' into wheelmap

6a2d2c3

paulmelnikow approved these changes Dec 10, 2018

View reviewed changes

PyvesB merged commit c893960 into badges:master Dec 10, 2018

PyvesB mentioned this pull request Dec 10, 2018

Accessibility status for locations via wheelmap.org #287

Closed

paulmelnikow mentioned this pull request Dec 21, 2018

Refactor [SymfonyInsight] to new service model and rename #2572

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Wheelmap] service rewrite and tests #2486

[Wheelmap] service rewrite and tests #2486

PyvesB commented Dec 8, 2018 •

edited

Loading

shields-ci commented Dec 8, 2018 •

edited

Loading

paulmelnikow left a comment

paulmelnikow Dec 8, 2018

paulmelnikow Dec 9, 2018

PyvesB Dec 9, 2018

paulmelnikow commented Dec 8, 2018

PyvesB commented Dec 9, 2018

paulmelnikow commented Dec 9, 2018 •

edited

Loading

PyvesB commented Dec 9, 2018

paulmelnikow commented Dec 9, 2018

PyvesB commented Dec 9, 2018

paulmelnikow commented Dec 9, 2018

PyvesB commented Dec 9, 2018 •

edited

Loading

PyvesB commented Dec 9, 2018 •

edited

Loading

shields-deployment bot commented Dec 10, 2018

[Wheelmap] service rewrite and tests #2486

[Wheelmap] service rewrite and tests #2486

Conversation

PyvesB commented Dec 8, 2018 • edited Loading

shields-ci commented Dec 8, 2018 • edited Loading

paulmelnikow left a comment

Choose a reason for hiding this comment

paulmelnikow Dec 8, 2018

Choose a reason for hiding this comment

paulmelnikow Dec 9, 2018

Choose a reason for hiding this comment

PyvesB Dec 9, 2018

Choose a reason for hiding this comment

paulmelnikow commented Dec 8, 2018

PyvesB commented Dec 9, 2018

paulmelnikow commented Dec 9, 2018 • edited Loading

PyvesB commented Dec 9, 2018

paulmelnikow commented Dec 9, 2018

PyvesB commented Dec 9, 2018

paulmelnikow commented Dec 9, 2018

PyvesB commented Dec 9, 2018 • edited Loading

PyvesB commented Dec 9, 2018 • edited Loading

shields-deployment bot commented Dec 10, 2018

PyvesB commented Dec 8, 2018 •

edited

Loading

shields-ci commented Dec 8, 2018 •

edited

Loading

paulmelnikow commented Dec 9, 2018 •

edited

Loading

PyvesB commented Dec 9, 2018 •

edited

Loading

PyvesB commented Dec 9, 2018 •

edited

Loading