feat: improve update_docs for openapi schema #2169
Conversation
drbh
changed the title
.github/workflows/autodocs.yaml
Outdated
| - name: Install Protoc | ||
| uses: arduino/setup-protoc@v1 | ||
| - name: Clean unused files | ||
| run: | | ||
| sudo rm -rf /usr/local/lib/android # will release about 10 GB if you don't need Android | ||
| sudo rm -rf /usr/share/dotnet # will release about 20GB if you don't need .NET |
There was a problem hiding this comment.
Where is that coming from ?Feels very random I don't think it' anywhere in our current CI.
There was a problem hiding this comment.
yea it is strange but was in tests.yml here
text-generation-inference/.github/workflows/tests.yaml
Lines 41 to 46 in 0759ec4
In general I'd love to avoid rebuilding the server in this workflow, but it may be needed for this check
| @@ -192,7 +190,6 @@ | |||
| "Text Generation Inference" | |||
| ], | |||
| "summary": "Generate a stream of token using Server-Sent Events", | |||
There was a problem hiding this comment.
Why are the descriptions gone ?
There was a problem hiding this comment.
this is related to an upstream change where utopia added support for a custom description instead of using the same value for both description and summary (related PRs juhaku/utoipa#781 and juhaku/utoipa#948).
| "example": "user" | ||
| "oneOf": [ | ||
| { | ||
| "$ref": "#/components/schemas/TextMessage" |
There was a problem hiding this comment.
Feels the cleaner types also help here, no ?
update_doc.py
Outdated
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
| parser = argparse.ArgumentParser( |
There was a problem hiding this comment.
Can we keep the simpler version that just checks everything by default ?
instead just add check_openapi function ?
There was a problem hiding this comment.
yep that makes sense, updated in the latest commits
update_doc.py
Outdated
|
|
||
| args = parser.parse_args() | ||
| process = subprocess.Popen( | ||
| ["text-generation-launcher"], |
There was a problem hiding this comment.
Can we use an extremely small model ?
There was a problem hiding this comment.
Maybe to simplify we could add a flag in router in order to print the openapi directly ?
Would remove the necessity to compile things (except the router).
I know I said internally I didn't like that option, but I also said after that you were right since I didn't have a better idea.
There was a problem hiding this comment.
updated and added a text-generation-router print-schema command that prints the api docs as JSON and exits, if no command is passed it goes through the normal flow
update_doc.py
Outdated
| json.dump(new_data, f, indent=2) | ||
|
|
||
|
|
||
| def compare_openapi(old_data, new_data): |
There was a problem hiding this comment.
Honestly I wouldn't even ever parse the files and simply reuse some simple diffing.
diff = subprocess.run(
["diff", tmp, filename], capture_output=True
).stdout.decode("utf-8")
print(diff)
Not strong opinion, your code seems to provide cleaner output, but given the unreadableness of the original JSON file, I'm not sure it's worth potentially making mistakes in that diff algorithm.
There was a problem hiding this comment.
good points! I think diff is a better long term solution and the pr has been updated to use diff
This PR aims to help sync the openapi schema
via a combination of pre-commit hooks and a new cli arg to dump the spec fileThis PR now updates
update_doc.pyto have two commands allowing callers to update the launcher docs and or the openapi specpython update_doc.py -h usage: update_doc.py [-h] {openapi,md} ... Update documentation for text-generation-launcher positional arguments: {openapi,md} openapi Update OpenAPI documentation md Update launcher and supported models options: -h, --help show this help message and exit