feat: Forward route class when it's different from APIRoute

alexschimpf / fastapi-versionizer

FastAPI Versionizer

MIT License

79 stars 13 forks source link

feat: Forward route class when it's different from APIRoute #53

Open yuripiratello opened 6 months ago

yuripiratello commented 6 months ago

Pull Request Checklist

[x] Make sure to read the contributor guide here.
[x] Make sure you are making a pull request against the main branch.
[x] Make sure your pull request title matches the requested format.
[x] Make sure to lint, type-check, and run unit and API tests.

Description

This PR enables the route_class to be forwarded to the versionizer API.

Resolve #52 .

alexschimpf commented 6 months ago

Looks like tests are failing

yuripiratello commented 6 months ago

Looks like tests are failing

Hey @alexschimpf !

The tests are failing on main in python versions: 3.9.18, 3.10.13, 3.11.7, and 3.12.1.

It's only working on v3.7.17. Same with this branch/PR.

Did I miss something from the setup steps? I don't think so... the single command required is make install-dev to run the tests.

alexschimpf commented 6 months ago

make install-dev will just install python requirements in your current virtualenv. I don't have anything fancy on local to test for all supported python versions. I've only added testing for multiple python versions via Github workflows.

I haven't looked into the errors that deeply, but... It's possible that the FastAPI versions are different between python versions, which is causing differences in test output. We may have to start dropping support for older python versions to be more in line with FastAPI. For example, FastAPI 0.104.0 dropped support for python 3.7.

yuripiratello commented 6 months ago

@alexschimpf, could you double-check on your machine if the main branch is passing on tests with a Python version greater than 3.7? Or rerun the last release actions? If you're looking for an easy way to test multiple python versions I would suggest using pyenv, it's really easy and straightforward to manage it, the steps that I executed were:

install any python version
create a virtualenv
activate
make install-dev
make run-tests

If you think a newer FastAPI version broke the lib, I can try to solve it, but I tested the lib with my project using Python 3.11, and it worked.

alexschimpf commented 6 months ago

@yuripiratello I get the same error as the github workflows show. But I'm running the tests against the main branch, which means the tests were broken before this PR. Not sure why you're not seeing errors? What version of fastapi do you have installed?

After some light testing, I see that the tests fail starting with fastapi==0.109.0. Here is the FastAPI 0.109.0 changelog notes: https://fastapi.tiangolo.com/release-notes/#01090

I don't really have time at the moment to look into this any more deeply. We could simply just pin the fastapi version to <=0.108.0 for now, until we figure out what was changed.

yuripiratello commented 6 months ago

@alexschimpf, I found the issue: It is the root_path on FastAPI. When you define this attribute, it appends the root_path as a prefix to the openapi_url. I created a new PR to fix that issue.

https://github.com/alexschimpf/fastapi-versionizer/pull/54

alexschimpf commented 6 months ago

@yuripiratello Can you merge in the latest main branch? That should fix the issue with the tests.

yuripiratello commented 6 months ago

Done @alexschimpf