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 for different API versions #22

Closed
MissiaL opened this issue Apr 10, 2020 · 21 comments
Closed

Support for different API versions #22

MissiaL opened this issue Apr 10, 2020 · 21 comments
Labels
enhancement New feature or request

Comments

@MissiaL
Copy link
Contributor

MissiaL commented Apr 10, 2020

There are different versions of API in my current project.
Like ​/api​/v{version}​/...
Is it possible to give different serializers depending on the version of api? I saw a similar implementation in the framework fastapi-versioning

Is there any possible solution to this problem? I will be very grateful for your advice

@MissiaL
Copy link
Contributor Author

MissiaL commented Apr 10, 2020

There is a similar problem in the drf-yasg, but it wasn’t solved.
axnsan12/drf-yasg#126

@MissiaL
Copy link
Contributor Author

MissiaL commented Apr 12, 2020

I still thought of a simpler option.
For example, use a version decorator and generate a schema using the version

    # Actual Api
    class CurentApiViewSet(viewsets.GenericViewSet):
        serializer_class = AlphaSerializer

        @extend_schema( )
        def create(self, request, *args, **kwargs):
            return Response({})

        @extend_schema()
        def list(self, request, *args, **kwargs):
            return Response([])

    class OldApiViewSet(viewsets.GenericViewSet):
        serializer_class = AlphaSerializer
        
        @version(1)
        @extend_schema( )
        def create(self, request, *args, **kwargs):
            return Response({})

        @version(1)
        @extend_schema()
        def list(self, request, *args, **kwargs):
            return Response([])

By default, generate a scheme only for methods that do not have a version decorator

To generate all versions, you need to pass a key that will generate a schema for each method separately

./manage.py spectacular --version=all

After generation, we get schemes with different versions of api, like schema_current.yml, schema_v1.yaml, ...
Then it would be possible to read these files from disk and give them locally. Or make some method to get the schema, for example, like here

class V1SpectacularAPIView(APIView):
    """
    OpenApi3 schema for this API. Format can be selected via content negotiation.
    - YAML: application/vnd.oai.openapi
    - JSON: application/vnd.oai.openapi+json
    """
    renderer_classes = [NoAliasOpenAPIRenderer, JSONOpenAPIRenderer]
    permission_classes = spectacular_settings.SERVE_PERMISSIONS

    @extend_schema(**SCHEMA_KWARGS)
    def get(self, request):
        generator_class = spectacular_settings.DEFAULT_GENERATOR_CLASS
        generator = generator_class(
            urlconf=spectacular_settings.SERVE_URLCONF,
            schema_version=1
        )
        return Response(generator.get_schema(
            request=request,
            public=spectacular_settings.SERVE_PUBLIC
        ))

@tsouvarev @tfranzel What do you think of this approach?

@tfranzel
Copy link
Owner

@MissiaL thanks for raising this. i already had a quick look into this a few weeks back but prioritized it down because its tricky and I was not quite sure what the best solution would look like.

you make an interesting point here. let me try to break it down

  • adding a param to SpectacularAPIView and ./manage spectacular would be the easiert part
  • lets say we would like to support URLPathVersioning and NamespaceVersioning because I assume yasg already did some serious thinking there at it was the most reasonable versioning to support.

when going down this road you have 2 options. duplicating the View for each version or build the versioning into get_serializer_class like the DRF doc shows https://www.django-rest-framework.org/api-guide/versioning/#varying-behavior-based-on-the-version . Maybe you would need to use both as the second one would only support differing serializers (and not different actions and such)

given all that there are 2 things to support i believe:

  1. have a "mock" request object providing the version attribute for automatic versions so that you can do if self.request.version == 'v1': in your get_serializer_class
  2. introduce a version scope to extend_schema. conceptually I already did this that before for action method scoping (https://github.com/tfranzel/drf-spectacular/blob/master/tests/test_regressions.py#L185).

for basic functionality 2. would be sufficient but 1. would be nice to have too.

for versioned schema output I assume this would be reasonable:

schema_noversion = unversioned (prob stuff like authentication and such)
schema_v1 = unversioned + v1
schema_v2 = unversioned + v2

Note: this is just an idea for now and i am not sure if it works out 100%

@MissiaL
Copy link
Contributor Author

MissiaL commented Apr 13, 2020

I like your idea. Thanks for this library!
I will wait for this patch

@tfranzel
Copy link
Owner

that was a tough one. We now support URLPathVersioning and NamespaceVersioning.

Here is how SpectacularApiView can be used:

# all unversioned
path('api/schema/', SpectacularAPIView.as_view()),
# manually versioned schema view that is in itself unversioned
path('api/schema-v2/', SpectacularAPIView.as_view(api_version='v2')),

If you do versioning on the schema view itself, it will honor that and only return the appropriate sub schema. I think that is pretty darn cool!

path('schema/', SpectacularAPIView.as_view(versioning_class=URLPathVersioning)),

@MissiaL let me know if that works correctly for you. the test cover a lot but its still tricky business.

@MissiaL
Copy link
Contributor Author

MissiaL commented May 10, 2020

Wow! This is a very cool addition! Thanks!

Unfortunately, this didn’t work on my project and now I don’t get the scheme at all 😢

Perhaps the fact is that we use a mixin to get the version
Our example:

def get_version_from_path(path: str) -> Optional[Tuple[int, ...]]:
    """
    Parses URL and returns version as tuple of ints
    """
    version_re = r'/api/v(?P<version>\d+.?\d*)/.*'
    match = re.match(version_re, path)
    if match:
        return tuple(map(int, match.group('version').split('.')))
    return None

class CustomRendererMixin:
    def get_renderers(self):
        # have to extract version directly from path because
        # this method is called long before
        # there any version is available from versioning class
        path = self.request._request.path
        version = get_version_from_path(path)
        if version and version > (2, 4):
            return [EnvelopedJSONRenderer()]
        return super().get_renderers()

class CommentViewSet(CustomRendererMixin, viewsets.GenericViewSet):
    ...

In our project, I did not find the use of classes

versioning_class = URLPathVersioning
versioning_class = NamespaceVersioning

@tsouvarev Could you help and tell in more detail how versioning works in our project?

@tfranzel
Copy link
Owner

That is unfortunate. In theory, the whole versioning should be completely inactive when no versioning class is specified.

Also, I don't really see your mixin interfering with the logic

Maybe a stacktrace would shed some light.

@MissiaL
Copy link
Contributor Author

MissiaL commented May 11, 2020

I analyzed the executable code and it seems I understood the reason

In our project, we use dynamic version number retrieval. The project does not use the version in include(...)
In the future I would like to study this issue in more detail.

Could you make the use of this setting optional? At the moment I get an empty schema 😿

@tfranzel
Copy link
Owner

i always try to avoid introducing further flags. could we check something different first?

i'm assuming you have a versioning class set on the view classes and it it is derived from the supported classes. is this what you do class YourVersioning(URLPathVersioning)?.

if you base your custom versioning class on BaseVersioning instead, the processing will be bypassed and the issue should disappear afais.

@MissiaL
Copy link
Contributor Author

MissiaL commented May 11, 2020

I found a problem in the code
https://github.com/tfranzel/drf-spectacular/blob/master/drf_spectacular/plumbing.py#L469

In our project, getting the version is different. We use custom class which return version in tuple

class CustomURLPathVersioning(URLPathVersioning):
    """
    Works similar to URLPathVersioning from DRF
    with exception that version is tuple of ints:
    public/api/v1.2/ -> version = (1, 2).
    String-based versions are harder to compare because of alphabetic ordering.
    """

    def determine_version(self, *args, **kwargs):
        version = super().determine_version(*args, **kwargs)
        return version and tuple(map(int, version.split('.')))

If I change this code, then it starts to work and I get a schema

def operation_matches_version(view, requested_version):
    try:
        version, _ = view.determine_version(view.request, **view.kwargs)
    except exceptions.NotAcceptable:
        return False
    else:
        if isinstance(version, tuple):
            version = str(version[0])
        return version == requested_version

Nevertheless, I still get an empty schema if I don't specify the version

@tfranzel
Copy link
Owner

tfranzel commented May 11, 2020

i completely forgot the command. as the cli can only take strings as args though. you can generate the schema with a "stringified" version like so:

./manage.py spectacular --file schema.yaml --api-version="(1, 2)"

that should work for you. pay attention to the space in the string. that parameter needs to be exactly what str(version) would output!

version matching is now based on str(x) == str(y), so you should get what you put in now.

Nevertheless, I still get an empty schema if I don't specify the version

so you mean empty paths? that might be as intended. are all your endpoints versioned?

schema_noversion = unversioned (prob stuff like authentication and such)
schema_v1 = unversioned + v1
schema_v2 = unversioned + v2

if nothing is unversioned the schema is defined empty, as expected. does that make sense?

@MissiaL
Copy link
Contributor Author

MissiaL commented May 11, 2020

that should work for you. pay attention to the space in the string. that parameter needs to be exactly what str(version) would output!

It is not working for my project, because I use custom versioning class

class CustomURLPathVersioning(URLPathVersioning):
    """
    Works similar to URLPathVersioning from DRF
    with exception that version is tuple of ints:
    public/api/v1.2/ -> version = (1, 2).
    String-based versions are harder to compare because of alphabetic ordering.
    """

    def determine_version(self, *args, **kwargs):
        version = super().determine_version(*args, **kwargs)
        return version and tuple(map(int, version.split('.')))

image

The old option for getting the schema was working.
Nevertheless, it seems to me that versioning could be made optional. This is really quite a complicated thing and may not work in some other projects.

@tfranzel
Copy link
Owner

i had another look. yes, it cannot work. the mechanic simply does not work with complex types.

i had a look at drf-yasg and here are my findings:

so i'm very sure having tuples as version is not working in yasg either. in my humble opinion, you are simply breaking the DRF interface there. All DRF native versioning classes work with strings. i acknowledge your need for the tuples, but it is simply not compatible with the other implementations. I'd like to stay close to what yasg is doing here.

i disabled all unsupported versioning classes with 51708cd. Since you do something custom, i recommend to derive from BaseVersioning with class CustomURLPathVersioning(BaseVersioning). That signals the class is doing something different and it will be bypassed by the versioning now. i hope that works for you.

@MissiaL
Copy link
Contributor Author

MissiaL commented May 11, 2020

Thanks for the fix. It almost works..
Is it possible to add to the function is_versioning_supported ignoring the version class to the function?
Or maybe it’s worth making the class definition more stringent like this

def is_versioning_supported(versioning_class):
    return (
        versioning_class is versioning.URLPathVersioning
        or versioning_class is versioning.NamespaceVersioning
    )

Thanks!

@tfranzel
Copy link
Owner

tfranzel commented May 11, 2020

i disabled all unsupported versioning classes with 51708cd. Since you do something custom, i recommend to derive from BaseVersioning with class CustomURLPathVersioning(BaseVersioning). That signals the class is doing something different and it will be bypassed by the versioning now. i hope that works for you.

i intend to support derived URLPathVersioning classes that adhere to the interface. you need to do this
class CustomURLPathVersioning(BaseVersioning)
instead of
class CustomURLPathVersioning(URLPathVersioning).

@MissiaL
Copy link
Contributor Author

MissiaL commented May 12, 2020

Yes. It works! Thanks for this feature!

@MissiaL MissiaL closed this as completed May 12, 2020
@tfranzel
Copy link
Owner

you're quite welcome. thank you for contributing!

sry you had to jump through those hoops. in the long run i believe it will pay off that we try to keep things as generic as possible.

@MissiaL
Copy link
Contributor Author

MissiaL commented May 13, 2020

Could you release a new version? 😃

@tfranzel
Copy link
Owner

sure thing. will happen in the next 1-2 hours

@tfranzel
Copy link
Owner

@MissiaL 0.9.4 is live 🎉

@laricko
Copy link

laricko commented Nov 29, 2023

In my opinion one of best aproach is

  • name your drf-spectacular settings like SPECTACULAR_SETTINGS_V1
  • pass to old api schema SpectacularJSONAPIView.as_view(custom_settings=SPECTACULAR_SETTINGS_V1)

and create your new api schema with new settings

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