DACCS-Climate / Marble-node-registry

The central node registry of the DACCS network
0 stars 1 forks source link

Allow alternate keywords for service #39

Open fmigneault opened 7 months ago

fmigneault commented 7 months ago

The current definition of "service" defines keywords as follows: https://github.com/DACCS-Climate/Marble-node-registry/blob/7fc410a0fa545c11540805db0adf2d7ad576fbf6/node_registry.schema.json#L96-L103

Given the specific values specified (especially service-[...] prefixed values), it feels more like a "type" than a generic keyword listing. Additional values cannot be provided either.

Should the definition allow extending with any other string, as long as at least 1 fulfills the requirement of the specific values?

mishaschwartz commented 6 months ago

As you point out, these are more "types" than "keywords". What if we changed this one to "type" and then had another "keyword" list that allowed any string?

fmigneault commented 6 months ago

Yes. That would work also. I was not sure about if it should be type: string or types: [string]? I think ["catalog", "data"] would be a common occurrence. What about other "types" that are not yet represented (e.g.: stac)? Finally, would the service- prefix be redundant, and should it be removed?

mishaschwartz commented 6 months ago

I was not sure about if it should be type: string or types: [string]? I think ["catalog", "data"] would be a common occurrence.

Yeah, we definitely should support multiple types.

What about other "types" that are not yet represented (e.g.: stac)?

STAC would be a "catalog" type right?

Finally, would the service- prefix be redundant, and should it be removed?

Sure that makes sense

fmigneault commented 6 months ago

STAC would be a "catalog" type right?

Yes. Good point.

So to summarize (other fields omitted just to be short) :

{
  "type": "object",
  "required": [
    "name",
    "types",
    "keywords",
    "description",
    "links"
  ],
  "properties": {
    "keywords": {
      "type": "array",
      "minItems": 1,
      "items": {
        "type": "string",
        "minLength": 1
      }
    },
    "types": {
      "type": "array",
      "minItems": 1,
      "items": {
        "type": "string",
        "enum": [
          "catalog",
          "data",
          "jupyterhub",
          "other",
          "wps",
          "wms",
          "wfs",
          "wcs",
          "ogcapi_processes"
        ]
      }
    }
  }
}
mishaschwartz commented 6 months ago

So to summarize

Look good, except for a few things:

fmigneault commented 6 months ago
  • Making types required will break backwards compatibility with existing service definitions.

    • I'm actually fine with this but it will break for PAVICS (they're behind a few versions of birdhouse-deploy)

Indeed. Should we introduce version-specific schemas? Instead of https://raw.githubusercontent.com/DACCS-Climate/Marble-node-registry/main/node_registry.schema.json#service the deployed instance should use https://raw.githubusercontent.com/DACCS-Climate/Marble-node-registry/1.1.0/node_registry.schema.json#service

Then, we can introduce 1.2.0 changes gradually into the respective services.

Do we want to support having no "keywords", either by not making it required or by not setting "minItems": 1,

I'm fine with removing that requirement.

mishaschwartz commented 6 months ago

Should we introduce version-specific schemas?

Yes for sure. That will help going forward but it won't really help with the backwards compatibility problem since the older ones are using main.

For that I'll probably have to make a conversion function in update.py that adds a default types value if it's not present.