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

Draft PR for Arazzo schema #198

Closed
wants to merge 3 commits into from
Closed

Draft PR for Arazzo schema #198

wants to merge 3 commits into from

Conversation

LasneF
Copy link
Member

@LasneF LasneF commented Jun 5, 2024

this is a draft PR, it contains a basic schema that validate the samples
it is mainly reproducing the structure of Arazzo

it is not complete and should not merge , it misses the reference mechanism that i will pick up from OAS schema

and also few points are set to object , but may be we can do better

@handrews
Copy link
Member

handrews commented Jun 5, 2024

@LasneF it would be better to use draft 2020-12 as that's the current draft being used in current versions of OAS and Arazzo. It's also a lot easier to re-use schemas (see the 3.0 vs 3.1 OAS schemas to see the difference, although I have no idea if it will make as much of a difference with Arazzo). With the OAS schemas, they use the same draft as is referenced in the spec they describe.

@handrews
Copy link
Member

handrews commented Jun 5, 2024

@LasneF also, thanks for starting this work! I should have said that before diving in with feedback 😅

@LasneF
Copy link
Member Author

LasneF commented Jun 6, 2024

I ve done a basis ,it s feature complete but with pending point
@handrews , @frankkilcommins could you a look and review especially the point below so that i fix them accordingly

How to modelize :

  • inputs that is describe as a Map[string, JSON Schema]
          "inputs": {
            "type": "object",
            "additionalProperties": {
              "type" : "object"
            },
            "description": "An object to hold reusable JSON Schema objects to be referenced from workflow inputs."
          },
  • I do not have modelize the runtime expression with a regex , I think it s ok i can mention a link in the description

  • when the specification mentionned that a field is a JSON schema it is not clear if it is inline or if it can be a $ref (that would be nice) , for now set as object

  • Re usable object ,
    I am not super confortable with the definition here
    https://spec.openapis.org/arazzo/latest.html#reusable-object

vs the OAI definition
https://spec.openapis.org/oas/latest.html#reference-object

the usage of the field reference toward $ref

for now skipping but it will be an easy addition then , i will follow the pattern if else
https://github.com/OAI/OpenAPI-Specification/blob/main/schemas/v3.1/schema.json#L350

and so for instance

parameters:
          - name: status
            in: query
            value: "available"
          - reference: $components.parameters.page
            value: 1
          - reference: $components.parameters.pageSize
            value: 10

I can make it compliant with either parameter = ( name & in ) Or Reference: + Value
with either this basic schema

  • RequestBody.payload is mentionned as any => i do not have put type

  • Criterion object

it s not 100 % clear what type : string | Criterion Expression Type Object , with criterion type object defined as type + version

it produces strange things like

- context: $statusCode
  condition: '^200$'
  type: regex

looks fair but if we consider type as an criterion expression object it gives

- context: $statusCode
  condition: '^200$'
  type: 
       type :  jsonpath 
       version :  draft-goessner-dispatch-jsonpath-00 

to me looks we should remove this notion and move the version in the fixed field ?
not sure still pending point . for now KISS approach just used

"type": {
            "type": "string", 
            "description": "The type of condition to be applied.",
            "enum" :  ["simple", "regex", "jsonpath", "xpath"]
          }

@Sintayew4
Copy link

#198

@jetro4u
Copy link

jetro4u commented Jul 25, 2024

 "inputs": {
            "type": "object",
            "additionalProperties": {
              "type" : "object"
            },
            "description": "An object to hold reusable JSON Schema objects to be referenced from workflow inputs."
          },

Just curious why do we need to add the 'additionalProperties' after defining "type": "object". Since the type is object, I think it can take any object type. By additionalProperties you probably mean attributes but I think that will be adding extra layer. But if it needs to be added, 'attributes' seems a better word.

@LasneF
Copy link
Member Author

LasneF commented Jul 29, 2024

@jetro4u you are right, dropped , as it more simple . i am not sure how to express that it has be conform to json schema , description may be good enough

@handrews
Copy link
Member

handrews commented Jul 29, 2024

@LasneF @jetro4u the additionalProperties: {type: object} would have been correct as far as it went if it were additionalProperties: {type: [object, boolean]}, as schemas can be objects or booleans and not any other type.

The fully correct way to handle this is shown in the OAS 3.1 schemas and relies on $dynamicRef and $dynamicAnchor.

@LasneF
Copy link
Member Author

LasneF commented Jul 30, 2024

@handrews i can reproduce the same model that is on the OAS 3.1 model
https://github.com/OAI/OpenAPI-Specification/blob/main/schemas/v3.1/schema.yaml#L726 howerver i do not know if

  1. it would be better to point to OAS schema (looks wrong to me as creating a strong dependency)
  2. use the same concept in arazzo spec . What about the $comment ? that is pointing to OAS spec
    2.1 would it be fair to keep it as a weak dependency to OAS project )
    2.2 just drop it as there is no such definition in Arrazzo spec
  3. just put type: additionalProperties: {type: [object, boolean]} , to keep it simple as for now no reuse

by the way thanks for pointing the OAS spec i realize that OAS did a great job by adding a $comment on most of the item will add it

@LasneF
Copy link
Member Author

LasneF commented Jul 31, 2024

closing this PR follow up is on #224

@LasneF LasneF closed this Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants