feedhenry / fh-fhc

FeedHenry CLI, the Command Line Interface to FeedHenry
Other
26 stars 66 forks source link

Fix args parsing - RHMAP-16759 #384

Closed evanshortiss closed 6 years ago

evanshortiss commented 7 years ago

Addresses #383

camilamacedo86 commented 7 years ago

Hi @evanshortiss, Tks for the contribution. Could you do the following actions for we are able to move forward with it?

  1. Could you rebase the PR with the master?
  2. Could you describe the steps to test it?
  3. Could you squash the both commits into just one as "JIRA - Text"?

After you do this steps I will test and if works I will approve your PR and merge it into the master.

evanshortiss commented 7 years ago

Thanks @camilamacedo86 - squash and rebase is done 😄

Steps for testing are available in issue #383, but I can provide more detail if required.

evanshortiss commented 7 years ago

Hmm, actually let me rephrase the steps in #383.

After this PR is merged we should be able to run commands from the CLI like so:

fhc projects read --project=$SOME_PROJECT_ID

and as a node.js script like so:

const fhc = require('fh-fhc')

fhc.load((err) => {
  if (err) throw err

  fhc.applyCommandFunction(['projects', 'read', '--project=SOME_PROJECT_ID'], console.log.bind(console))
})
camilamacedo86 commented 7 years ago

Hi @evanshortiss,

This PR is working for the example that you wrote here as follows.

fhc.applyCommandFunction(['projects', 'read', '--project=hgxe4z6k3qxgpolko4ry7fik'], console.log.bind(console));

However, it is not working when is required more than one parameter as follows.

fhc.applyCommandFunction(['env', 'list', '--app=hgxe4z5j2b6owln5vw6qvx5i --env=dev'], console.log.bind(console));

Result:

{ [Error: Error - not 2xx status code. (400)
{"message":"expected a guid :invalid guid char: [ ]","status":"error","result":null}] requestId: '1a38e534-0a35-4f11-ad34-b981ee8d55ef' }
fhc ERR! cb() never called!
fhc ERR!  
fhc Command executed with error

Also, it is not solving the situation where the V3 standard can not receive a sequence of an ordered array as I understood that was your intention here as follows. ( Make the V3 standard calls accept to be as for the old standard )

    fhc.env.list({_:['hgxe4z5j2b6owln5vw6qvx5i', 'dev'], json : true }, function (err,res) {
      return cb( err);
    });

Result:

fhc ERR! Error: Error - not 2xx status code. (400)
fhc ERR! {"message":"expected a guid :Invalid guid length, 9 = 4*2+1, only +2 or +3 allowed","status":"error","result":null}
fhc ERR! Request ID:  8eaa9d93-c741-479a-bde5-7dd97b7fc6b7
fhc ERR! 
fhc ERR! System Darwin 15.6.0
fhc ERR! command "/Users/cmacedo/.nvm/versions/node/v4.3.0/bin/node" "/Users/cmacedo/work/fh-fhc/bin/fhc.js" "test"

The tests made was with the command fhc env list --app=<app> [--env=<env>] [--json=<json>] Following the result of this command into the command line.

dhcp-17-108:fh-fhc cmacedo$ ./bin/fhc.js env list hgxe4z5j2b6owln5vw6qvx5i dev true
[{
  "businessObject": "cluster/reseller/customer/domain/project/cloud-apps/environment-variable",
  "domain": "support",
  "appId": "hgxe4z5j2b6owln5vw6qvx5i",
  "varName": "asdasd",
  "varValues": {
    "dev": "adasd"
  },
  "masked": false,
  "guid": "kfoo52j2hegdtnxip2u7jdyh",
  "sysOwner": "i4ze7ruomd27wnohth22to4v",
  "sysVersion": 0,
  "sysCreated": 1501702889102,
  "sysModified": 1501702889114,
  "sysGroupFlags": 65567,
  "sysGroupList": ""
}]
dhcp-17-108:fh-fhc cmacedo$ 

Following how we are expecting that it be called as a dependency which is working.

    fhc.env.list({app:'hgxe4z5j2b6owln5vw6qvx5i', env:'dev', json : true }, function (err,res) {
      return cb( res);
    });

Could you check these tests? If it was not your intention could you describe it for us? Please, feel free to ping me and we check it together.

camilamacedo86 commented 7 years ago

@evanshortiss I tried to test as you can check above. Could you validate it? Please, let me know if I did something wrong or if you need some help in order to check it for we move forward.

camilamacedo86 commented 6 years ago

@evanshortiss could verify the review? Is it something that we should still consider or could we close this one?

camilamacedo86 commented 6 years ago

@evanshortiss we are looking for your feedback here. Should we keep this PR and issue open or we can close them? If it is to be kept open could you check the review made?

camilamacedo86 commented 6 years ago

Consing this PR since it is not so long passing in the Jenkins, CI process and shows outdated without any interaction for a long time.