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

Render API docs #6463

Merged
merged 48 commits into from
Mar 2, 2024
Merged

Render API docs #6463

merged 48 commits into from
Mar 2, 2024

Conversation

SchrodingersGat
Copy link
Member

@SchrodingersGat SchrodingersGat commented Feb 10, 2024

Render API schema as part of documentation.

  • A snapshot of each schema will be available at each release, and also for "latest" (master)
  • API schema file available for download (.yaml file)

References

@SchrodingersGat SchrodingersGat added enhancement This is an suggested enhancement or new feature api Relates to the API documentation labels Feb 10, 2024
@SchrodingersGat SchrodingersGat added this to the 0.14.0 milestone Feb 10, 2024
Copy link

netlify bot commented Feb 10, 2024

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
🔨 Latest commit c9ebdbd
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/65e26b4d62d07a0008f0f0f2

Copy link
Member

@matmair matmair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable. Is the api supposed to be rendered already?

docs/docs/api/schema.md Outdated Show resolved Hide resolved
docs/docs/api/schema.md Outdated Show resolved Hide resolved
SchrodingersGat and others added 4 commits February 11, 2024 07:49
Co-authored-by: Matthias Mair <code@mjmair.com>
Co-authored-by: Matthias Mair <code@mjmair.com>
- seems to render more reliably
@SchrodingersGat
Copy link
Member Author

Seems reasonable. Is the api supposed to be rendered already?

It did not seem to be rendering reliably with that library. I have tried a new one - check it out now and see what you think

}

if SITE_URL:
SPECTACULAR_SETTINGS['SERVERS'] = [SITE_URL]
Copy link
Contributor

@martonmiklos martonmiklos Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The url subkey is missing here:

Optional list of servers.

# Each entry MUST contain "url", MAY contain "description", "variables"
# e.g. [{'url': 'https://example.com/v1', 'description': 'Text'}, ...]
'SERVERS': [],

Apart from this it is working fine: I had been able to generate code with openapi-codegen without any hacking now!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can also use this then to auto generate a ts client (fully
Typesafe) for PUI (and also publish as npm package for the users) and a python client to eventually replace the InvenTree-Python package to reduce maintainance tasks there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stuff needed for that is in #6440

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The url subkey is missing here:

Optional list of servers.

# Each entry MUST contain "url", MAY contain "description", "variables"
# e.g. [{'url': 'https://example.com/v1', 'description': 'Text'}, ...]
'SERVERS': [],

Apart from this it is working fine: I had been able to generate code with openapi-codegen without any hacking now!

Thanks, should be fixed now :)

docs/api.yml Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having an auto-generated file here will create a lot of fun with merge conflicts and blow up the repo size, we have had this discussion multiple times in the last year with frontend resources and I pointed this out 2 days ago.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only a WIP to ensure that we are all happy with the generated output. Once that is all clear, I'll setup a build pipeline to render this file on docs build, rather than actually committing the file itself

@SchrodingersGat
Copy link
Member Author

The generated schema file (.html) is 16MB - which might take some time to load. Do we think this is an issue?

@wolflu05
Copy link
Contributor

This is really laggy to open with a browser. How about splitting it in multiple pages categorized by the Django apps, so each apps api has a sub page. This reduces the download size and also the page size.

@SchrodingersGat
Copy link
Member Author

@wolflu05 nice idea - any idea how to achieve that?

@wolflu05
Copy link
Contributor

Not really sure about how to support that, the mkdocs plugin seems like it's not. So we would need to either find a way to export the schema for each app individually or write a script to divide the schema into multiple pieces and then generate multiple pages that use the OAD plugin.

@SchrodingersGat
Copy link
Member Author

or write a script to divide the schema into multiple pieces and then generate multiple pages that use the OAD plugin.

Looking into this approach, doesn't seem too bad...

@SchrodingersGat SchrodingersGat mentioned this pull request Feb 12, 2024
9 tasks
@wolflu05
Copy link
Contributor

Performance and load speed is much better now, thank you really much. Maybe it's worth putting the machine api endpoints into its own page, but I'm happy with it now.

@matmair
Copy link
Member

matmair commented Feb 19, 2024

@wolflu05 seems like the machine API is throwing an error and not building https://github.com/inventree/InvenTree/actions/runs/7959068634/job/21725283385?pr=6463#step:4:40

@SchrodingersGat
Copy link
Member Author

@wolflu05 seems like the machine API is throwing an error and not building https://github.com/inventree/InvenTree/actions/runs/7959068634/job/21725283385?pr=6463#step:4:40

@matmair why is the CI passing then - the API schema errors should cause CI to fail...

@SchrodingersGat
Copy link
Member Author

@wolflu05 I have split machines out into their own path now

@wolflu05
Copy link
Contributor

wolflu05 commented Feb 20, 2024

Thank you.

This error is interesting, because there is actually a correct schema annotation with extend_schema.

@matmair
Copy link
Member

matmair commented Feb 20, 2024

@SchrodingersGat as far as I can read the CI runs the API schema task failed and threw an error

@wolflu05
Copy link
Contributor

Yes, but not the generation step. That passed. The difference checking step is the step that failed. Has that something todo with the ongoing gh actions beta where there are bugs from time to time like this?

@matmair
Copy link
Member

matmair commented Feb 20, 2024

It fails within the CI job, I think that is enough

@wolflu05
Copy link
Contributor

I see, but why does the ci does not fail on master then? The machine api is already merged in there.

@SchrodingersGat
Copy link
Member Author

@matmair @wolflu05 any ideas how we should proceed here? This is the final blocker for the 0.14.0 release

@matmair
Copy link
Member

matmair commented Feb 27, 2024

AFAIK there is a schema issue and a problem with translations; I have just restarted the docs CI - maybe it runs now

@SchrodingersGat
Copy link
Member Author

@matmair any chance you can have a look at this? I am still a bit confused by the CI process here

@wolflu05
Copy link
Contributor

@SchrodingersGat The issue occurs while exporting the schema. A requests serializer is missing for introspection in one of the machine API serializers. However this doesn't throw an error, but instead the schema.yml file is not created, so a later step expecting this file failed because it cannot find it.

Regarding the machine api error it seems like this can be fixed with this patch:

diff --git a/InvenTree/machine/api.py b/InvenTree/machine/api.py
index 9448d2ea9..2fec1b4cc 100644
--- a/InvenTree/machine/api.py
+++ b/InvenTree/machine/api.py
@@ -144,7 +144,7 @@ class MachineRestart(APIView):
 
     permission_classes = [permissions.IsAuthenticated]
 
-    @extend_schema(responses={200: MachineSerializers.MachineRestartSerializer()})
+    @extend_schema(request=None, responses={200: MachineSerializers.MachineRestartSerializer()})
     def post(self, request, pk):
         """Restart machine by pk."""
         machine = get_machine(pk)

And to already fail in the export step, we may could check for the existence of the file and if not exit 1.

@SchrodingersGat SchrodingersGat changed the title [WIP] Render API docs Render API docs Mar 2, 2024
@SchrodingersGat SchrodingersGat merged commit 39ba25c into inventree:master Mar 2, 2024
23 checks passed
@SchrodingersGat SchrodingersGat deleted the api-docs branch March 2, 2024 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Relates to the API documentation enhancement This is an suggested enhancement or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants