Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix NovaFlavor , add @JsonIgnore to Boolean isPublic() #820

Merged
merged 1 commit into from
Sep 27, 2016

Conversation

zizhongwei
Copy link
Contributor

fix NovaFlavor , add @JsonIgnore to Boolean isPublic()
issues : #818

@auhlig
Copy link
Member

auhlig commented Sep 20, 2016

@zizhongwei Why ignoring the attribute? From what I can see here https://github.com/ContainX/openstack4j/blob/master/core/src/main/java/org/openstack4j/openstack/compute/domain/NovaFlavor.java#L45 only os-flavor-access:is_publicis being fetched and just publicis ignored anyway, isn't it?

@zizhongwei
Copy link
Contributor Author

In class NovaFlavor , the get method named "isPublic()" returns a Boolean value. when the object of this class was converted to JSON ,the JSON String will have a property 'public' ,But we also added the @JsonProperty("os-flavor-access:is_public") on the same property ,so the converted JSON String will have two property : "public" & "os-flavor-access:is_public" in JSON String, but the property : "public" is unexpected ;like this:
image

when use this JSON String to call the create API,it will retunt BAD_REQUEST :
https://cloud.githubusercontent.com/assets/12456254/18656483/ffe6947c-7f23-11e6-8d31-6566af35377b.png

so we need to add @JsonIgnore on "Boolean isPublic()" to remove the unexpected property 'public' and remain the property "os-flavor-access:is_public" in the JSON String

@auhlig
Copy link
Member

auhlig commented Sep 22, 2016

Mh. Not quite sure where the public is coming from as the annotation cleary says os-flavor-access:is_public but your explanation sound reasonable. What do you think @vinodborole?

@vinodborole
Copy link
Contributor

same thought @auhlig need more time to investigate on this, what do you think?

@auhlig
Copy link
Member

auhlig commented Sep 22, 2016

Any volunteers? ;)

@biogerm
Copy link

biogerm commented Sep 23, 2016

We have seen same problem here.

@biogerm
Copy link

biogerm commented Sep 23, 2016

LGTM

@biogerm
Copy link

biogerm commented Sep 23, 2016

Sep 23, 2016 5:23:41 PM org.glassfish.jersey.filter.LoggingFilter log
INFO: 1 * Sending client request on thread main
1 > POST https://10.33.252.104:8774/v2.1/flavors
1 > Accept: application/json
1 > Content-Type: application/json
1 > User-Agent: OpenStack4j / OpenStack Client
1 > X-Auth-Token: gAAAAABX5UfQgQ0O8yVqgMQPY5E7M4fiWPpMv250h8rs0jspjjt7tgirdm3IRhnqbeM7Bn2AP3zmYOBI4prsit0PYS5ELOO4b1zBLLN_YiJPy2sXrgQ8o-9MUFCpXzcssXoSzWJzgQcIcRQrqrR6L1UaRztTkjqPH9UX0Yh68NDtKfHG4Tn5eyM
{
  "flavor" : {
    "name" : "testFlavor",
    "ram" : 4096,
    "vcpus" : 1,
    "disk" : 30,
    "swap" : 0,
    "public" : true,
    "OS-FLV-EXT-DATA:ephemeral" : 0,
    "rxtx_factor" : 1.0,
    "os-flavor-access:is_public" : true
  }
}

Sep 23, 2016 5:23:41 PM org.glassfish.jersey.filter.LoggingFilter log
INFO: 1 * Client response received on thread main
1 < 400
1 < Connection: close
1 < Content-Length: 343
1 < Content-Type: application/json; charset=UTF-8
1 < Date: Fri, 23 Sep 2016 15:23:41 GMT
1 < Vary: X-OpenStack-Nova-API-Version
1 < X-Compute-Request-Id: req-e9357d30-467f-40e2-9f1d-00695c1e421c
1 < X-Openstack-Nova-Api-Version: 2.1
{"badRequest": {"message": "Invalid input for field/attribute flavor. Value: {u'name': u'testFlavor', u'ram': 4096, u'vcpus': 1, u'swap': 0, u'os-flavor-access:is_public': True, u'rxtx_factor': 1.0, u'OS-FLV-EXT-DATA:ephemeral': 0, u'disk': 30, u'public': True}. Additional properties are not allowed (u'public' was unexpected)", "code": 400}}

@vinodborole
Copy link
Contributor

@biogerm thanks for volunteering 👍 LGTM @auhlig I think we can merge this, unless you have anything

@auhlig
Copy link
Member

auhlig commented Sep 27, 2016

Thanks for the work @biogerm .
I agree @vinodborole. Good to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants