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

log fhconfig #130

Closed
wants to merge 1 commit into from
Closed

log fhconfig #130

wants to merge 1 commit into from

Conversation

odra
Copy link
Contributor

@odra odra commented Sep 26, 2018

Jira link(s)

https://issues.jboss.org/browse/RHMAP-21672

What

fh-mbaas is failing to retrieve the cloud app's connection string.

Why

Issue is caused by incorrect usage of fh-config api.

How

It doesn't make much sense to use the reload method: https://github.com/feedhenry/fh-config/blob/master/lib/fhconfig.js#L108 when retrieving the connection string since we were using an empty array as its function argument.

Verification Steps

  • Deploy the fh-mbaas generated by this PR;
  • Create a blank project in studio and import the cloud app from this repo: https://github.com/feedhenry/testing-cloud-app;
  • Deploy the cloud app and trigger a test by accessing its "/test" route;
  • All tests should be green.

Checklist:

  • Code has been tested locally by PR requester
  • Changes have been successfully verified by another team member

@odra odra requested a review from matskiv September 26, 2018 17:11
@odra
Copy link
Contributor Author

odra commented Sep 26, 2018

I will bump the component's version once the PR gets approved.

Copy link
Contributor

@matskiv matskiv left a comment

Choose a reason for hiding this comment

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

I would rather not do this change. This code is proven by time and removal might have some unexpected consequences.
I found the root cause and posted a comment on JIRA, let's solve based on that. @odra wdyt?

@@ -102,6 +102,8 @@ function loadConfig(cb) {
// read our config file
var configFile = process.env.conf_file || args._[0];

console.log(fhconfig);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be merged?

@@ -102,6 +102,8 @@ function loadConfig(cb) {
// read our config file
var configFile = process.env.conf_file || args._[0];

console.log(fhconfig);

fhconfig.init(configFile, requiredvalidation, function(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shows that @matskiv has reason the name of the function reload was changed in the 2.0.0 of the dep fh-config. Also, shows that for we move forward with it instead of you use the init here we could use the reloadRawConfig.

@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Sep 27, 2018

Hi @odra

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.

3 participants