Skip to content
This repository has been archived by the owner on May 17, 2024. It is now read-only.

feat: support on templating env for route inputs #8

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

blaise-favor
Copy link
Contributor

@blaise-favor blaise-favor commented Jul 4, 2023

Problem/Issue:

Supporting templating ENV for input in hub-functions. Instead of sending in secrets etc via payload or query, we need to support templating, with handlebars.

Trello issue here

Proposed Solution:

Implement interpolation mechanism from environment variables using handlebars library for http request inputs.

app.js Fixed Show fixed Hide fixed
@blaise-favor blaise-favor changed the title feat: support on env templating for route inputs feat: support on templating env for route inputs Jul 4, 2023
app.js Outdated
console.log('FATAL', 'Set NODE_ENV to production or development or add', configPath);
throw new Error(msg);
}
throw new Error(err.message);

Choose a reason for hiding this comment

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

Suggested change
throw new Error(err.message);
throw err;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To remove block. We won't be using config files instead as per discussion.

app.js Outdated
Comment on lines 11 to 21
const cwd = process.cwd();
const NODE_ENV = process.env.NODE_ENV || 'development';
const CONFIG_BASE_PATH = process.env.CONFIG_BASE_PATH || './config';
const configFileName = `${NODE_ENV}.json`;

let configPath;
if (path.isAbsolute(CONFIG_BASE_PATH)) {
configPath = path.normalize(CONFIG_BASE_PATH) + path.sep + configFileName;
} else {
configPath = CONFIG_BASE_PATH + '/' + configFileName;
}

Choose a reason for hiding this comment

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

Is this necessary? If we really have to validate the config, I'd rather we check the values of what exp-config returns than just checking whether the config file exists since the values are more important.

I suggest removing this file validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To remove block. We won't be using config files instead as per discussion.

@@ -14,6 +16,8 @@ exports.plugin = {
name: 'ldap',
register: async function (server) {
server.ext('onRequest', (request, h) => {
helpers.setQueryParameterTemplateValues(request.query, config.ldap);

Choose a reason for hiding this comment

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

Suggested change
helpers.setQueryParameterTemplateValues(request.query, config.ldap);
helpers.setQueryParameterTemplateValues(request.query, config.ldap);

Is request.query at this point an object or still the serialized string value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

request.query is already an object at this point

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