-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Adding support for Starlette Request and Responses #104
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
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for Starlette Request and Response types to the Azure Functions extensions by creating new packaging, implementation, and test files. Key changes include:
- Adding a new pyproject.toml for packaging the Starlette-based extension.
- Implementing the core HTTP extension functionality in the azurefunctions/extensions/http/starlette directory.
- Adding tests and updating package initialization files.
Reviewed Changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
azurefunctions-extensions-http-starlette/pyproject.toml | Introduces packaging configuration for the new Starlette extension. |
azurefunctions-extensions-http-starlette/tests/init.py | Sets up the test bootstrap for the extension. |
azurefunctions-extensions-http-starlette/README.md | Provides user documentation, though it currently contains inconsistencies with naming. |
azurefunctions-extensions-http-starlette/azurefunctions/extensions/http/starlette/init.py | Exposes Starlette-based request/response and web server classes. |
azurefunctions-extensions-http-starlette/tests/test_web.py | Contains tests for the web module and synchronization functionality. |
azurefunctions-extensions-http-starlette/azurefunctions/extensions/http/starlette/web.py | Implements the core functionality for Starlette based request/response handling and server setup. |
azurefunctions-extensions-http-starlette/azurefunctions/extensions/http/init.py, azurefunctions-extensions-http-starlette/azurefunctions/init.py, azurefunctions-extensions-http-starlette/azurefunctions/extensions/init.py | Package initializations using pkgutil to extend package paths. |
Files not reviewed (2)
- azurefunctions-extensions-http-starlette/LICENSE: Language not supported
- azurefunctions-extensions-http-starlette/MANIFEST.in: Language not supported
Comments suppressed due to low confidence (4)
azurefunctions-extensions-http-starlette/README.md:1
- The header incorrectly references a FastApi library; please update it to 'Starlette' to match the PR's purpose.
# Azure Functions Extensions Http FastApi library for Python
azurefunctions-extensions-http-starlette/README.md:4
- The source code link points to the FastApi extension repository; update the URL to reference the Starlette extension repository.
[Source code](https://github.com/Azure/azure-functions-python-extensions/tree/main/azurefunctions-extensions-http-fastapi)
azurefunctions-extensions-http-starlette/README.md:20
- The package name in the requirements should be updated to 'azurefunctions-extensions-http-starlette' to reflect the actual extension implemented.
3. Add ```azurefunctions-extensions-http-fastapi``` to your requirements.txt
azurefunctions-extensions-http-starlette/README.md:21
- Update the import statement to use
azurefunctions.extensions.http.starlette
instead of fastapi so that it aligns with the new implementation.
4. Import Request and different types of responses from ```azurefunctions.extensions.http.fastapi``` in your HttpTrigger functions.
| [Samples](hhttps://github.com/Azure/azure-functions-python-extensions/tree/main/azurefunctions-extensions-http-fastapi/samples) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the URL ('hhttps'); it should be corrected to 'https'.
| [Samples](hhttps://github.com/Azure/azure-functions-python-extensions/tree/main/azurefunctions-extensions-http-fastapi/samples) | |
| [Samples](https://github.com/Azure/azure-functions-python-extensions/tree/main/azurefunctions-extensions-http-fastapi/samples) |
Copilot uses AI. Check for mistakes.
@@ -0,0 +1,74 @@ | |||
# Azure Functions Extensions Http FastApi library for Python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the FastApi references to Starlette as well as any other Starlette specific README changes
"FileResponse" | ||
] | ||
|
||
__version__ = "0.0.1b1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Version can be 1.0.0b1
[project] | ||
name = "azurefunctions-extensions-http-starlette" | ||
dynamic = ["version"] | ||
requires-python = ">=3.10" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FastAPI supports 3.8+. Is there a reason starlette has to be 3.10+?
] | ||
dependencies = [ | ||
'azurefunctions-extensions-base', | ||
'azure-functions', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can azure-functions
be removed as a dependency?
self.assertEqual( | ||
ResponseTrackerMeta.get_response_type(ResponseLabels.FILE), | ||
StarletteFileResponse, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a test_code_quality.py
test file
# Copyright (c) Microsoft Corporation. All rights reserved. | ||
# Licensed under the MIT License. | ||
|
||
"""Bootstrap for '$ python setup.py test' command.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is legacy from using setup.py -- it can be removed
self.assertEqual( | ||
ResponseTrackerMeta.get_response_type(ResponseLabels.FILE), | ||
StarletteFileResponse, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the pipelines to run these tests. You will need a ci-starlette-test.yml
file under eng/ci
and a starlette-unit-tests.yml
file under eng/templates/official/jobs
self.assertEqual( | ||
ResponseTrackerMeta.get_response_type(ResponseLabels.FILE), | ||
StarletteFileResponse, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once the extension is published on PyPI, a samples
directory will also need to be added
@@ -0,0 +1,24 @@ | |||
__path__ = __import__("pkgutil").extend_path(__path__, __name__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this
@@ -0,0 +1,54 @@ | |||
[build-system] | |||
requires = ["setuptools >= 61.0", "wheel"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is wheel also needed here?
'pytest-instafail', | ||
'pre-commit', | ||
'isort', | ||
'black' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add flake8
and mypy
here? Those are used for our code quality tests
@@ -0,0 +1,225 @@ | |||
import asyncio |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the copyright text to the top of this
No description provided.