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

optimize setup_openapi #84

Closed
chobeat opened this issue Jun 12, 2020 · 6 comments · Fixed by #86
Closed

optimize setup_openapi #84

chobeat opened this issue Jun 12, 2020 · 6 comments · Fixed by #86
Assignees
Labels
openapi Issue or pull request related to OpenAPI code perf A code change that improves performance
Milestone

Comments

@chobeat
Copy link

chobeat commented Jun 12, 2020

Problem: Using rororo with aiohttp, I end up reloading the openapi.yaml for every test and populating the app with setup_openapi. This takes up 30% of the total time of my test suite. 15% is just loading the file in read_schema, while another 12% is spent in create_spec.

Proposal: decouple the spec creation from the addition of fields and routes to the app. After this, create a separate setup_openapi (maybe called setup_test_openapi) that can take a Spec object as an additional parameter and set it instead of creating it. The underlying implementation will be the same, but the first half of the current setup_openapi will be optional

I can try to work on this if you don't see conflicts arising

@playpauseandstop
Copy link
Owner

Hi @chobeat

Thanks for your question, I’m curious whether this trick helps you: https://rororo.readthedocs.io/en/latest/openapi_speedups.html#testing-cache-reading-schema-and-spec-creation

Here the example in rororo test suite:

def test_cache_create_schema_and_spec(schema_path):

In my projects passing cache_create_schema_and_spec=True helps save around 40-45% on running tests

@playpauseandstop
Copy link
Owner

As of initial proposal of passing spec instance directly in setup_openapi I also thought about that many times, so maybe I will need to implement that option as well

Thanks again for starting discussion about the topic

@playpauseandstop playpauseandstop added openapi Issue or pull request related to OpenAPI code perf A code change that improves performance labels Jun 12, 2020
@playpauseandstop playpauseandstop added this to the 2.0 milestone Jun 12, 2020
@playpauseandstop playpauseandstop self-assigned this Jun 12, 2020
@chobeat
Copy link
Author

chobeat commented Jun 12, 2020

Oh, it looks totally fine. I was using an older version of rororo and I must have missed that parameter. I will try it for sure, thanks.

@playpauseandstop
Copy link
Owner

Great!

But I believe directly passing Spec instance to setup_openapi function instead of Path is completely valid usecase

And shouldn’t take long to implement and test

@chobeat
Copy link
Author

chobeat commented Jun 12, 2020

I tried your suggestion: it goes faster but I think I've found an unrelated bug. In unmarshallers.py:198 it tries to access the keys of a None. From what I understand, it's doing recursion on nested dicts but it tries on the None value and it fails. This has been introduced between 2.0.0b3 and 2.0.0rc2. Before it was working properly.

I think it's happening on all my responses that contain at least a None value somewhere.

playpauseandstop added a commit that referenced this issue Jun 14, 2020
…up_openapi`

Previously *rororo* relies only on `schema_path` positional argument and
does read schema from path and instantiate spec instance inside of
`setup_openapi` call.

This commit allows to directly path `schema` and `spec` keyword args to
`setup_openapi`, which gives developers wider range of caching / reusing
schema & spec for environments, which needs to instantiate
`web.Application` (and call `setup_openapi`) a lot.

Fixes: #84
@playpauseandstop
Copy link
Owner

Hi @chobeat,

Your initial proposal on passing spec into setup_openapi call fulfilled in #86

After it will be merged into master, you'll be able to,

from pathlib import Path

import yaml
from aiohttp import web
from openapi_core.shortcuts import create_spec
from rororo import setup_openapi


openapi_yaml = Path(__file__).parent / "openapi.yaml"
schema = yaml.load(
    openapi_yaml.read_bytes(), Loader=yaml.CSafeLoader
)
spec = create_spec(schema)

app = setup_openapi(
    web.Application(), 
    operations,
    schema=schema,
    spec=spec,
)

Also, in contradiction, with your initial request, you must pass schema dict to setup_openapi as well, as rororo internally needs to access raw schema for,

  • Handing OpenAPI schema via GET /api/openapi.yaml or GET /api/openapi.json
  • Deciding which server to use by iterating over servers list

playpauseandstop added a commit that referenced this issue Jun 14, 2020
…up_openapi` (#86)

Previously *rororo* relies only on `schema_path` positional argument and
does read schema from path and instantiate spec instance inside of
`setup_openapi` call.

This commit allows to directly path `schema` and `spec` keyword args to
`setup_openapi`, which gives developers wider range of caching / reusing
schema & spec for environments, which needs to instantiate
`web.Application` (and call `setup_openapi`) a lot.

Fixes: #84
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openapi Issue or pull request related to OpenAPI code perf A code change that improves performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants