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

Add REST API docs #70

Merged
merged 8 commits into from
Oct 18, 2023
Merged

Conversation

pavithraes
Copy link
Member

@pavithraes pavithraes commented Oct 12, 2023

Closes #69

(We should add an action to autogenerate the openapi.json file during releases.)

@pmeier
Copy link
Member

pmeier commented Oct 12, 2023

Not blocking, but a few changes need to be made:

image

  • Instead of FastAPI in the header, we should probably have "Ragna" there and maybe later even our logo.
  • Not sure what the "default" does there. Can we remove it?
  • The "Try it out" button needs to be removed. For the docs of a live deployment this is fine, but since we are in static environment here, there is nothing for the user to try.

Copy link
Member

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

(We should add an action to autogenerate the openapi.json file during releases.)

I would take it a step further and not commit this file at all, but rather generate it every time we run the doc build similar to what we are planning for the CLI docs. We can use mkdocs hooks for this. Will have a go at it later this week.

@pmeier
Copy link
Member

pmeier commented Oct 12, 2023

Here is a script that creates the openapi.json file

import contextlib
import subprocess
import sys
import time

import httpx

import ragna


def main():
    with api() as url:
        response = httpx.get(f"{url}/openapi.json")

    if not response.is_success:
        raise RuntimeError("Unable to retrieve openapi.json from running ragna api")

    with open("openapi.json", "wb") as file:
        file.write(response.content)


@contextlib.contextmanager
def api(timeout=10, poll=0.5):
    process = subprocess.Popen(
        [sys.executable, "-m", "ragna", "api", "-c", "ragna.demo_config"]
    )

    url = ragna.demo_config.ragna_api_url
    try:
        start = time.time()
        while (time.time() - start) < timeout:
            with contextlib.suppress(httpx.ConnectError):
                response = httpx.get(f"{url}/health")
                if response.is_success:
                    yield url
                    return

            try:
                process.communicate(timeout=poll)
            except subprocess.TimeoutExpired:
                # Timeout expiring is good here, because that means the process is still
                # running
                pass
            else:
                break

        raise RuntimeError("Unable to start ragna api")
    finally:
        process.kill()
        process.communicate()


if __name__ == "__main__":
    main()

EDIT: While the script above works, it is also possible to generate the schema without starting the API (fastapi/fastapi#1173 (comment)). Since that is way faster, I went with it instead.

Copy link
Member

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

@pavithraes I took the liberty and pushed a solution as outlined in #70 (review). Now the openapi.json file is automatically regenerated whenever we build the documentation.

I've also fixed the title, i.e. FastAPI -> ragna, as mentioned in #70 (comment). The other comments are also worth addressing, especially the "Try it out" one. But we can also open an issue and do it later.

@pmeier pmeier added area: documentation 📖 Improvements or additions to documentation and removed pr-status: needs review 👀 labels Oct 12, 2023
@pmeier
Copy link
Member

pmeier commented Oct 12, 2023

If I'm right about #68 (comment), this PR will close #68.

@pmeier pmeier linked an issue Oct 12, 2023 that may be closed by this pull request
ragna/_api/core.py Outdated Show resolved Hide resolved
@pavithraes
Copy link
Member Author

pavithraes commented Oct 17, 2023

@pmeier - Thanks for your updates to this PR. I'm trying to test this locally, but I see AttributeError: 'Rag' object has no attribute 'get_logger' on running mkdocs serve.

I did merge the recent updates on main here, am I missing something?

Full traceback:
INFO    -  Building documentation...
Traceback (most recent call last):
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/bin/mkdocs", line 8, in <module>
    sys.exit(cli())
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/lib/python3.9/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/lib/python3.9/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/lib/python3.9/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/lib/python3.9/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/lib/python3.9/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/lib/python3.9/site-packages/mkdocs/__main__.py", line 270, in serve_command
    serve.serve(**kwargs)
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/lib/python3.9/site-packages/mkdocs/commands/serve.py", line 86, in serve
    builder(config)
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/lib/python3.9/site-packages/mkdocs/commands/serve.py", line 67, in builder
    build(config, live_server=None if is_clean else server, dirty=is_dirty)
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/lib/python3.9/site-packages/mkdocs/commands/build.py", line 280, in build
    config.plugins.on_pre_build(config=config)
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/lib/python3.9/site-packages/mkdocs/plugins.py", line 530, in on_pre_build
    return self.run_event('pre_build', config=config)
  File "/Users/pavithraes/opt/anaconda3/envs/ragna-dev/lib/python3.9/site-packages/mkdocs/plugins.py", line 509, in run_event
    result = method(**kwargs)
  File "/Users/pavithraes/Developer/Quansight/ragna/docs/hooks/rest_api.py", line 15, in on_pre_build
    app = api(Rag(load_components=False))
  File "/Users/pavithraes/Developer/Quansight/ragna/ragna/_api/core.py", line 38, in api
    rag = Rag(config)
  File "/Users/pavithraes/Developer/Quansight/ragna/ragna/core/_rag.py", line 30, in __init__
    self._logger = self.config.get_logger()
AttributeError: 'Rag' object has no attribute 'get_logger'

@pmeier
Copy link
Member

pmeier commented Oct 17, 2023

I did merge the recent updates on main here, am I missing something?

There was a change in the backend that needed to also be done in the REST API doc hook that I implemented. I've added the change and you should be able to build locally now.

@pavithraes
Copy link
Member Author

Thanks! This works. :)

@pavithraes pavithraes merged commit 9abe0b8 into Quansight:main Oct 18, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: documentation 📖 Improvements or additions to documentation type: enhancement 💅 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DOC] Create REST API docs [DOC] Workflow to generate reference docs
2 participants