Enhancement: Add Python 3.12 #159
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ansion Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…rough an exception Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
jenniferjiangkells
left a comment
jenniferjiangkells
left a comment
There was a problem hiding this comment.
The approach looks sound to me. The most important thing is that it's backward compatible with the cookbook examples and sets up a solid foundation for future improvements, and this ticks the box.
I've made some general comments, but tbh the only change absolutely needed is removing spyne as a dependency in the pyproject.toml.
| endpoints_registry[service_name].add(f"WSGI:{mount_path}") | ||
| logger.debug(f"Registered WSGI service {service_name} at {mount_path}") | ||
| logger.info(f"🔧 Registering {service_name} at: {mount_path}") | ||
| logger.info(f" Router type: {type(router_or_app)}") |
There was a problem hiding this comment.
Maybe make these logs debug instead of info
| # Use include_router for APIRouter instances | ||
| if isinstance(router_or_app, APIRouter): | ||
| if hasattr(router_or_app, "routes"): | ||
| logger.info(f" Routes in router: {len(router_or_app.routes)}") |
| # For FastAPI apps, use mount | ||
| self.mount(mount_path, router_or_app) | ||
| endpoints_registry[service_name].add(f"MOUNTED:{mount_path}") | ||
| logger.info(f"✅ Mounted app {service_name} at {mount_path}") |
There was a problem hiding this comment.
filename convention is with no underscore
There was a problem hiding this comment.
preferred convention for tests is flat structure without classes
| "starlette>=0.40.0,<0.42.0", | ||
| "uvicorn>=0.24.0,<0.25", | ||
| "httpx>=0.27.0,<0.28", | ||
| "spyne>=2.14.0,<3", |
There was a problem hiding this comment.
Remove the spyne dependency here
|
|
||
|
|
||
| @dataclass | ||
| class ServerFault(Exception): |
There was a problem hiding this comment.
The fault models defined here are not actually used (built manually in fastapi_server.py). Consider removing or using them in the router - just keep consistent.
| actual_location = f"{base_url}{path}" | ||
|
|
||
| # Replace the location in WSDL | ||
| wsdl_content = wsdl_content.replace( |
There was a problem hiding this comment.
These are hardcoded - consider using a template variable like {{SERVICE_LOCATION}} in WSDL and replace that
Description
This PR adds functionality for python 3.12
Address #127
This is achieved by removing Spyne as a dependency for SOAP servers.
In particular, we add a FastAPI server and a WSDL file while removing spyne and associated files.
Changes Made
Python 3.12 is added as an acceptable python version. The ci/cd will now additionally run on 3.12 as a test version. The publishing of the package will also be done on 3.12
Testing
Vibe coded tests were run, existing tests were modified to run on new code.
Additionally the Cookbook example for connecting to Medplum was confirmed to run and give the same output before and after.
Checklist