Skip to content
This repository has been archived by the owner on Jul 4, 2019. It is now read-only.

Consolidate api gateway urls and keys #22

Merged
merged 2 commits into from
Sep 20, 2017
Merged

Conversation

thurstontye
Copy link
Contributor

@thurstontye thurstontye commented Sep 19, 2017

  • Use API_GATEWAY_HOST in place of ACS_API_URL, ALS_API_URL, and HUI_BASE_URL
  • Use one config var KAT_MEMB_API_KEY for requests to ALS and ACS as they are the same key.
  • Remove empty emailNotification.js file

🐿 v2.5.16

Tye, Thurston added 2 commits September 19, 2017 13:12
…SE_PATH

Use one config var API_MEMB_GATEWAY_KEY for requests to ALS and ACS as they are the same key.
Remove empty emailNotification.js file
 🐿 v2.5.16
Copy link
Contributor

@leggsimon leggsimon left a comment

Choose a reason for hiding this comment

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

Faaaaaantastic!!! This is awesome! 💥

@@ -10,7 +10,7 @@ const logger = require('@financial-times/n-logger').default;
const env = require('./helpers/env');
const expectOwnProperties = require('./helpers/expectExtensions').expectOwnProperties;
const mockAPI = env.USE_MOCK_API;
const baseUrl = `${require('./../lib/helpers/config').ACS_API_URL}/acquisition-contexts`;
const baseUrl = `${require('./../lib/helpers/config').API_GATEWAY_HOST}/acquisition-contexts`;
Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of consistency could we make this

const config = require('./../lib/helpers/config');
const baseUrl = `${config.API_GATEWAY_HOST}/acquisition-contexts`;

API_GATEWAY_HOST: envVars.KAT_API_GATEWAY_HOST || envVars.API_GATEWAY_HOST,
API_GATEWAY_KEY: envVars.KAT_API_GATEWAY_KEY || envVars.API_GATEWAY_KEY,
KAT_MEMB_API_KEY: envVars.KAT_MEMB_API_KEY || envVars.KAT_ALS_API_KEY || envVars.ALS_API_KEY,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we now just remove KAT_ALS_API_KEY and ALS_API_KEY completely??

@thurstontye thurstontye merged commit 74ceba9 into master Sep 20, 2017
@thurstontye thurstontye deleted the config-clean-up branch September 26, 2017 11:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants