Open Api to Hydra Parser basic implementation #197
Conversation
vaibhavchellani
changed the title
| baseURL = doc["host"] | ||
| name = doc["basePath"] | ||
| api_doc = HydraDoc(name, title, desc, name, baseURL) | ||
| definitions = doc["definitions"] |
There was a problem hiding this comment.
Definitions won't always be defined in doc["definitions"]. You should use $ref see this https://swagger.io/docs/specification/using-ref/
There was a problem hiding this comment.
yes you are right , right now i am first parsing definitions and adding props and classes to api doc , then i parse paths and check if the path has the same name as class , if yes i add the operations .
Would the following be a better implementation , parse the paths , when $ref is found , parse whatever is defined and create classes ?
There was a problem hiding this comment.
Yeah, you should check where $ref points and then extract the definitions from there. It can be in the same doc or at some remote location but for now using local definitions should be fine
| properties = definitions[class_]["properties"] | ||
| classAndClassDefinition[class_] = classDefinition | ||
| definitionSet.add(class_) | ||
| api_doc.add_supported_class(classDefinition, collection=False) |
There was a problem hiding this comment.
How are you checking if any endpoint is a collection or not?
There was a problem hiding this comment.
I am not right now , in our discussions we decided to do that later .
| try: | ||
| parameters = paths[path][method]["parameters"] | ||
| for param in parameters: | ||
| op_expects = param["schema"]["$ref"].split('/')[2] |
There was a problem hiding this comment.
Should be 'vocab:'+param["schema"]["$ref"].split('/')[2]. ALso what if the parameter is something as simple as an integer and it's not defined in schema?
For example -
parameters:
- name: petId
in: path
description: ID of pet to return
required: true
type: integer
format: int64There was a problem hiding this comment.
I have planned on adding some datatypes which Open api support like string , integer etc as classes and add those when we encounter them . I am adding the "vocab" part later in line 94
| # dirty hack , do case insensitive search more gracefully | ||
| possiblePath = possiblePath.replace(possiblePath[0], possiblePath[0].upper()) | ||
| # check if the path name exists in the classes defined | ||
| if possiblePath in definitionSet and len(path.split('/')) == 2: |
There was a problem hiding this comment.
What about paths like /user/createWithList or any other path with length > 2?
There was a problem hiding this comment.
yes , want to discuss about what to do about those .
| "vocab:" + op_expects, | ||
| op_returns, | ||
| op_status)) | ||
| api_doc.add_supported_class(classAndClassDefinition[possiblePath], collection=False) |
There was a problem hiding this comment.
Again, will only work for paths like /User. Also, what about collection type?
Important - You're adding same class twice to the doc. First in line 41 and then again here.
Why?
There was a problem hiding this comment.
there would be some classes which are only made to be referenced later on , classes are added when their parsing is done , later if we find operations on them we add the operations and add it to the api doc again , the class appears only once in the final doc . Please suggest a better way .
| paths = doc["paths"] | ||
| definitionSet = set() | ||
| classAndClassDefinition = dict() | ||
| hydra_doc = "" |
There was a problem hiding this comment.
Please avoid using so many Global variables. You're not using function arguments at all.
It's not a good practice.
There was a problem hiding this comment.
okay , thank you , i agree , will do .
|
|
||
| """ | ||
| import yaml | ||
| from doc_writer import HydraDoc, HydraClass, HydraClassProp, HydraClassOp |
There was a problem hiding this comment.
Make sure you use absolute imports, not relative imports.
|
@xadahiya can this PR be merged ? |
|
@chrizandr ^^ |
|
@vaibhavchellani reduce the number of global variables a bit. Rest everything looks fine. |
|
@xadahiya @chrizandr done . |
There was a problem hiding this comment.
Please make the changes suggested. Also, have you tried running a hydrus instance with the generated API documentation?
You can replace the API doc in used in hydrus.tests with the newly generated one and see if you get any errors. It's a good way to check if you missed something out. Please check that and let us know what you find.
| print(exc) | ||
|
|
||
|
|
||
| definitionSet = set() |
There was a problem hiding this comment.
Declare these in the getClasses() function and pass it as a variable to get_ops(). Whenever you can replace global variables with passable parameters, you should opt for the latter. hydra_doc can be declared in getClasses() as well and can be returned from to main when function finishes execution.
| import yaml | ||
| import json | ||
|
|
||
| from hydrus.hydraspec.doc_writer import HydraDoc, HydraClass, HydraClassProp, HydraClassOp |
|
|
||
| from hydrus.hydraspec.doc_writer import HydraDoc, HydraClass, HydraClassProp, HydraClassOp | ||
|
|
||
| with open("../samples/openapi_sample.yaml", 'r') as stream: |
| except KeyError: | ||
| op_returns = None | ||
| classAndClassDefinition[possiblePath].add_supported_op(HydraClassOp(op_name, | ||
| op_method, |
There was a problem hiding this comment.
Methods are lower case in OpenAPI, but in Hydra, the standard is to use upper case method names. Convert "put"/"post"/"get"/"delete" to "PUT"/"POST"/"GET"/"DELETE". This is also messing up the proper @type for the supported operations.
There was a problem hiding this comment.
yeah my bad , will correct this .
| definitionSet.add(class_) | ||
| api_doc.add_supported_class(classDefinition, collection=False) | ||
| for prop in properties: | ||
| new_prop = HydraClassProp("vocab:" + prop, prop, required=False, read=True, write=True) |
There was a problem hiding this comment.
maintaining line length < 80 according to PEP8, will be better.
|
@xadahiya @chrizandr @Shekharrajak fixed most of the things pointed out |
|
can this be merged ? |
|
Merging #197 |
Fixes #193 and #198
Checklist
Description
This script is able to parse the open api spec and is able to extract the classes , supported props , supported operations from the file and create a Hydra Api Documentation .
Test Logs
@xadahiya @chrizandr @Shekharrajak , let me know the changes , its WIP