Azure / open-service-broker-azure

The Open Service Broker API Server for Azure Services
https://osba.sh
MIT License
248 stars 101 forks source link

Convert "location" string property to enum in JSON schema #655

Closed pkosiec closed 5 years ago

pkosiec commented 5 years ago

Description Currently, the location property in schemas of various Service Plans, like Basic Tier plan for Azure Database for MySQL 5.7, is defined as simple string property. However, using enum in this case would be a huge improvement for usability in rendering the JSON schema as UI form.

It is related to JSON schema rendering on UI, similar to #651 and #519.

Example Basic Tier plan for Azure Database for MySQL 5.7

As this is a simple string property, user has to type the location manually:

screen shot 2018-12-18 at 10 07 41

Expected result Beta plan for "Cloud SQL - MySQL" Service Class (Google Cloud Platform Broker)

As this is enum, it renders as convenient select box:

screen shot 2018-12-18 at 09 33 20 screen shot 2018-12-18 at 09 33 25
norshtein commented 5 years ago

Hi @pkosiec , thanks for raising this! This proposal looks pretty good to us. Could you give us an example JSON object so that we can refactor our code?

pkosiec commented 5 years ago

Hi @norshtein,

Sure thing. Instead of the following location property object:

{
    "location": {
      "description": "The Azure region in which to provision applicable resources.",
      "title": "Location",
      "type": "string"
    }
}

I would like to see this one:

{
  "location": {
    "default": "eastasia",
    "enum": [
      "eastasia", "southeastasia", "centralus", "eastus", "eastus2", "westus", "northcentralus", "southcentralus", "northeurope", "westeurope", "japanwest", "japaneast", "brazilsouth", "australiaeast", "australiasoutheast", "southindia", "centralindia", "westindia", "canadacentral", "canadaeast", "uksouth", "ukwest", "westcentralus", "westus2", "koreacentral", "koreasouth", "francecentral", "francesouth", "australiacentral", "australiacentral2"
    ],
    "description": "The Azure region in which to provision applicable resources.",
    "title": "Location",
    "type": "string"
  }
}

I don't know if I included all available locations, but anyway, here's the general concept how it could look like 🙂

norshtein commented 5 years ago

Hi @pkosiec , the example does make sense. OSBA will apply this change soon.

pkosiec commented 5 years ago

Thank you very much, @norshtein!

derberg commented 5 years ago

@norshtein Hey, thanks a lot for such rapid activity in those issues that Pawel reported.

Could you explain what you mean in statement that OSBA will apply this change soon ? What OSBA has to do with that? OSBA only requires the broker provider to specify a valid v4 json schema https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#input-parameters-schema-object so both usaceses on providing enum as array of strings or array of objects as part of oneOf are valid as they are valid with Json Schema. Or I'm totally missing something and I apologize in advance 🙈

pkosiec commented 5 years ago

@derberg I think OSBA in this context means Open Service Broker Azure - the same as the name of this repo 🙂 At least that's how I understood the comment.

derberg commented 5 years ago

Interesting, @norshtein OSBA is for you Open Service Broker API or Open Service Broker Azure 😄

pkosiec commented 5 years ago

@derberg The official abbreviation of Open Service Broker API is OSBAPI (search for it here: https://www.openservicebrokerapi.org/ or on https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md), and abbreviation of Open Service Broker for Azure is OSBA (search for OSBA on main readme: https://github.com/Azure/open-service-broker-azure).

Case solved - I think your concerns are not valid anymore 😄

norshtein commented 5 years ago

Hi @derberg , my OSBA in previous comment means Open Service Broker Azure. What @pkosiec said is correct.