Skip to content
This repository has been archived by the owner on Jun 28, 2021. It is now read-only.

Handle missing config file #80

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

tomzaoral
Copy link

Use default values from test config and prevent error if you don't have access rights to HOME dir (e.g. Heroku) and therefore config file was not properly created during installation of bitpay-rest library.

Use default values from test config and prevent error if you don't have access rights to HOME dir (e.g. Heroku) and therefore config file was not properly created during installation of bitpay-rest library.
@kleetus
Copy link
Contributor

kleetus commented Jul 14, 2016

I am generally ok with this solution except that the non-existence of a config.json will lead to the assumption that the user wants to use https://test.bitpay.com:443 as the defaultConf object. This may not be the case and the user may not realize what host is being used. This could lead to bad things happening depending on the use case. Maybe a better solution is to read the config from a static json file that is included with the npm module -or- from an AWS file -or- a response from an api request. But, you do bring up a good point and the solution seems simple enough under a narrow band of circumstances, but I think we need explicit input on what the defaultConf object is under all circumstances.

@ecejas
Copy link

ecejas commented Jul 23, 2019

Can someone merge this? In the age of serverless computing and the cloud seems counterintuitive to force the existence of a config file on the local "server".

@ecejas
Copy link

ecejas commented Jul 23, 2019

Better yet would be to load the config file only if exists. Fail if there is not config file and not override given.

Comment on lines +14 to +18
var defaultConf = {
"apiHost": "test.bitpay.com",
"apiPort": 443,
"forceSSL": true
}

Choose a reason for hiding this comment

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

To avoid duplicating default values already present elsewhere, I suggest using this approach instead:

Suggested change
var defaultConf = {
"apiHost": "test.bitpay.com",
"apiPort": 443,
"forceSSL": true
}
var defaultConf = require('../config/test.json');

See: #92 (comment)

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.

5 participants