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

Support OpenAPI spec generation [ENHANCEMENT] #778

Open
pandu-k opened this issue Mar 5, 2024 · 6 comments
Open

Support OpenAPI spec generation [ENHANCEMENT] #778

pandu-k opened this issue Mar 5, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@pandu-k
Copy link
Collaborator

pandu-k commented Mar 5, 2024

Is your feature request related to a problem? Please describe.
In Marqo 2.x there isn't a way to generate OpenAPI spec. This was possible from the /openapi.json path.

Describe the solution you'd like
An endpoint, usable from a running Marqo container, from which to retrieve the openapi specs.

Additional context
Relevant docs: https://fastapi.tiangolo.com/reference/openapi/docs/

@pandu-k pandu-k added the enhancement New feature or request label Mar 5, 2024
@gabauer
Copy link
Contributor

gabauer commented Sep 19, 2024

Hi @pandu-k,

What's the current status of this issue? It seems like the OpenAPI spec generation is still not functional, as I'm unable to retrieve it from the expected /openapi.json endpoint.

Additionally, the documentation in the README might need an update, as it implies that generating the OpenAPI spec is still supported, which doesn't seem to be the case.

Could you provide any updates or guidance on how to proceed? Thanks!

@gabauer
Copy link
Contributor

gabauer commented Sep 19, 2024

I’ve found a solution that restores OpenAPI spec generation and resolves the issue with the /openapi.json and /docs paths. The root cause was in src/marqo/tensor_search/models/add_docs_objects.py, where the documents field included a union type. The issue arose because the second type of the union, np.ndarray, could not be serialized properly.

From my perspective, there are a few potential ways to address this:

  1. Implement a custom OpenAPI function for FastAPI to exclude ndarray fields from the schema.
  2. Create a custom JSON encoder for models containing ndarray.
  3. Replace np.ndarray with a standard Python list type.

I opted for the third approach since it was the simplest and most straightforward solution.

Here are the specific changes I made:

  • On line 33, I updated documents: Union[Sequence[Union[dict, Any]], np.ndarray] to documents: Union[Sequence[Union[dict, Any]], list].

  • On line 71, I changed docs: Union[Sequence[Union[dict, Any]], np.ndarray] to docs: Union[Sequence[Union[dict, Any]], list].

After applying these modifications, the OpenAPI spec at /openapi.json and Swagger UI at /docs are now functioning as expected.

I'm not entirely sure if this is the ideal long-term solution, but if it seems like a good fit, I’d be happy to submit a PR for it.

@pandu-k
Copy link
Collaborator Author

pandu-k commented Sep 19, 2024

Thanks @gabauer - we'll look into this

@papa99do
Copy link
Collaborator

Hello @gabauer , thanks for looking into this and raising a PR. I checked the source code and noticed that the ndarray type here is not used in any code or tests. I think it would be safe to remove it. Actually, the only type accepted for docs is List[Dict[str, Any]]

@gabauer
Copy link
Contributor

gabauer commented Sep 23, 2024

Hi @papa99do, thanks for looking into it, I will check this out, try to let the tests run and add a test with a simple "is there" check on the openapi URIs to prevent breaking this stuff in the future.

@papa99do
Copy link
Collaborator

papa99do commented Nov 8, 2024

Hello @gabauer , sorry for the long wait. I've merged in your change after addressing the build permission issue. I've tested that the openAPI endpoints can be accessed again. I'll follow up to add more documentation and examples in following PRs. Thanks again for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants