prosociallearnEU / cf-nodejs-client

A Cloud Foundry Client for Node.js
https://prosociallearneu.github.io/cf-nodejs-client/
Apache License 2.0
17 stars 41 forks source link

Adding ServiceInstances model and tests. #142

Closed jthomas closed 8 years ago

jthomas commented 8 years ago

One more! I've added the ServiceInstances API model along with the tests. I've tested it against Bluemix including creating and deleting new instances. Re-factor as you see fit.

jabrena commented 8 years ago

Nice Agreement! :) https://www.youtube.com/watch?v=G_Sy6oiJbEk

Tomorrow, I will check in local and I will do test again.

Many thanks.

Cheers from Madrid

jabrena commented 8 years ago

Hi James,

I have just tested the code.

Why do you need this feature? With services, you receive the service but what is the usage of a instance Service?

Juan Antonio

jthomas commented 8 years ago

I'm working on a project (https://github.com/jthomas/cfbot) that monitors the CF Events API.

One of the events (audit.service_binding.create) provides the metadata reference to _service_instanceguid.
I need to use this key to find the label property from the corresponding service for my application.

"entity": {
            "type": "audit.service_binding.create",
            "actor": "09192995-5f15-4ffc-85dc-5313124f09ab",
            "actor_type": "user",
            "actor_name": "james.thomas@uk.ibm.com",
            "actee": "98290303-51d0-4879-8bb4-39d75042a9d8",
            "actee_type": "service_binding",
            "actee_name": "",
            "timestamp": "2015-12-15T15:48:48Z",
            "metadata": {
               "request": {
                  "service_instance_guid": "9de4d44b-7ef0-4c17-84c4-4e8245703531",
                  "app_guid": "531bf4bb-3496-4574-b314-8336b50976eb"
               }
            },
            "space_guid": "596ac566-45b7-4472-b33a-3d1d836dd289",
            "organization_guid": "0a551850-a9b1-4036-8c06-88a3b7b8fe28"
         }
jabrena commented 8 years ago

I notice that you are using ES2015 in the syntax. Maybe in next releases we could improve the syntax too.

Nice project.

jabrena commented 8 years ago

Your idea about cfbot is to share information about events from CF in order to improve QoS service using Slack as Team tool, isn't it?

jthomas commented 8 years ago

Yeah ES2015 is awesome! I created it to help distributed teams understand who's making changes to apps everyones working on and for monitoring apps that might get into trouble.

jabrena commented 8 years ago

Good morning James, v0.12.0 is out.

npm outdate
npm update

For next minor version, I would like to upgrade the library to ES2015 besides, I would like to refactor the library to set the token in a property and avoid passing the credential in every method.

What is your opinion?

Merry Christmas.

JAB

jthomas commented 8 years ago

Thanks for letting me know, I'll update my client library.

:+1: on passing the tokens in a property, it'll make the service creation boilerplate much cleaner. Good idea.

Feliz Navidad!

jabrena commented 8 years ago

Yes, now the development is stable so, It is necessary to reduce the number of lines of the usage of this library. Besides, I will upgrade to ES6. I was reading your code a bit on https://github.com/jthomas/cfbot and I think that I will update it too :)

I did a mini POC here: https://github.com/prosociallearnEU/cf-nodejs-client/blob/master/lib/utils/HttpUtils.js

Besides, I have added ESLint.

Play a bit with the scripts:

npm run lint

Now, you will observe several warnings about Camel case rule, but I will remove them in the next code upgrade.

Merry Christmas James.

Cheers

jabrena commented 8 years ago

If you like, give me a chance to finish this issues and I will do and example and later update your classes, ok?

I am trying to finish these issues first: https://github.com/prosociallearnEU/cf-nodejs-client/issues/148 https://github.com/prosociallearnEU/cf-nodejs-client/issues/149 https://github.com/prosociallearnEU/cf-nodejs-client/issues/150

jthomas commented 8 years ago

Looking good! I've updated CFBot and it's working without any issues. Cheers for keeping me updated.

jabrena commented 8 years ago

Good morning @jthomas,

I have just uploaded the code in a stable way. If you observe Travis results: https://travis-ci.org/prosociallearnEU/cf-nodejs-client

You will see that I have enabled ESLint as a previous task to test the project. In the case of the classes: https://github.com/prosociallearnEU/cf-nodejs-client/blob/master/lib/model/cloudcontroller/ServiceInstances.js https://github.com/prosociallearnEU/cf-nodejs-client/blob/master/lib/model/cloudcontroller/ServicePlans.js https://github.com/prosociallearnEU/cf-nodejs-client/blob/master/lib/model/cloudcontroller/Services.js

The changes are very easy:

  1. Extend class from CloudControllerBase
  2. Change http code from String to Int using the properties from HttpCodes. You have an example here: https://github.com/prosociallearnEU/cf-nodejs-client/blob/master/lib/model/cloudcontroller/Domains.js all codes are here: https://github.com/prosociallearnEU/cf-nodejs-client/blob/master/lib/utils/HttpCodes.js in the whole project the http codes uses are: 200, 201 & 204
  3. Remove in all methods the need to send the token parameters:
  106:43  warning  Identifier 'token_type' is not in camel case                     camelcase
  106:55  warning  Identifier 'access_token' is not in camel case                   camelcase
  1. change that parameters to guid to simplify. It is very rare to use 2 guid in CC method.

Example: from servicePlan_guid to guid

 84:86  warning  Identifier 'servicePlan_guid' is not in camel case               camelcase

5. Don't worry for this ESLint warning:

``` bash
/home/travis/build/prosociallearnEU/cf-nodejs-client/lib/model/cloudcontroller/ServiceInstances.js
   42:5   warning  All "var" declarations must be at the top of the function scope  vars-on-top
/home/travis/build/prosociallearnEU/cf-nodejs-client/lib/model/cloudcontroller/ServicePlans.js
   42:5   warning  All "var" declarations must be at the top of the function scope  vars-on-top
/home/travis/build/prosociallearnEU/cf-nodejs-client/lib/model/cloudcontroller/Services.js
   42:5   warning  All "var" declarations must be at the top of the function scope  vars-on-top

to run ESLint is easy:

npm run lint

Tell me if you have some problem.

Cheers

jthomas commented 8 years ago

Thanks for the feedback. I'll try to get to updating these classes later in the week, but feel free to do this if you have time.