`no_proxy` BayModel attr type

Bug #1533145 reported by Jamie Hannaford
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Magnum
Triaged
Wishlist
Madhuri Kumari

Bug Description

Why is the `no_proxy` attr a comma-separated string?

It is defined here:

https://github.com/openstack/magnum/blob/master/magnum/api/controllers/v1/baymodel.py#L113

And here is an example:

https://github.com/openstack/magnum/blob/master/magnum/api/controllers/v1/baymodel.py#L182

Surely an array of strings would be a better and more appropriate JSON data structure?

Revision history for this message
wangqun (bjwqun) wrote :

I think that the comma-separated string can be easier when dealing with the backend of magnum. So I don't suggest to change the type of "no_proxy".

Revision history for this message
Jamie Hannaford (jamie-hannaford) wrote :

I'm not sure that API decisions should work in the favour of internal developers.

The type *is* a collection of strings, not one big concatenated one. I don't see a good reason to confuse the clarity of the types here.

Revision history for this message
Thomas Maddox (thomas-maddox) wrote :
Revision history for this message
Thomas Maddox (thomas-maddox) wrote :

I agree with Jamie here. Why use a special format for a list when that type already exists? If there's some special case where the backend requires it be a comma-separated string, then a `no_proxy.join(",")` can handle that at a conversion layer before the backend receives it. There it can be cleanly separated and documented as to why the backend needs it this way.

Revision history for this message
Kyle Kelley (rgbkrk) wrote :

If it's a publicly exposed interface, we need to rely on JSON for consumption by SDKs, clients, and other tooling that will use Magnum.

Adrian Otto (aotto)
Changed in magnum:
status: New → Triaged
importance: Undecided → Low
Changed in magnum:
assignee: nobody → Madhuri Kumari (madhuri-rai07)
Changed in magnum:
importance: Low → Wishlist
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers