Conversation
… Dockerfile, tests, the entire works
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Markdownlint (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Prospector (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Pylint (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Pylintpython3 (reported by Codacy) found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
There was a problem hiding this comment.
Pull request overview
This PR migrates the service from Flask to FastAPI, modernizes packaging/tooling around uv + pyproject.toml, and updates the test suite/CI to match the new runtime and API behavior.
Changes:
- Replaced Flask app/blueprints with a FastAPI app factory, routers, and Pydantic models.
- Switched packaging from
setup.py/requirements.txttopyproject.toml(Hatchling) and addeduv-driven Make targets + Docker image. - Reworked unit tests to use
fastapi.testclient.TestClientand added expanded utility/API parameter coverage.
Reviewed changes
Copilot reviewed 34 out of 41 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| unit/test_marvel.py | Updates Marvel endpoint tests for FastAPI redirects/response handling; adds query-param tests. |
| unit/test_file.py | Adds unit coverage for ReadFile data loading/filtering/sorting. |
| unit/test_factory.py | Adds FastAPI factory/settings tests (currently includes unused imports). |
| unit/test_error.py | Adds tests for InvalidUsage error object behavior. |
| unit/test_dc.py | Updates DC endpoint tests for FastAPI redirects/response handling; adds query-param tests. |
| unit/test_common.py | Adds tests for common utility helpers (direction/param normalization/permutations). |
| unit/test_api.py | Adjusts help tests to use FastAPI response interfaces and updated endpoints. |
| unit/conftest.py | Switches fixtures from Flask test client to FastAPI TestClient. |
| tests/test_factory.py | Removes legacy Flask-based tests from tests/. |
| tests/README.md | Documents that tests/ is reserved for a Postman/API integration submodule. |
| tests/.this_is_a_git_submodule | Placeholder marker file for the reserved submodule directory. |
| setup.py | Removes legacy setuptools packaging entrypoint. |
| setup.cfg | Removes legacy pytest/coverage configuration (moved to pyproject.toml). |
| service/utils/file.py | Adds typing and refactors i18n sorting into a dedicated helper. |
| service/utils/error.py | Improves typing/docs for InvalidUsage and keeps JSON-serializable output. |
| service/utils/common.py | Adds typing and clarifies utility class docs. |
| service/utils/api.py | Refactors config normalization and adds typing; retains shorthand flag behavior. |
| service/settings.py | Removes old dotenv-loading module (replaced by pydantic-settings). |
| service/routes/swagger.py | Removes Flask swagger UI wiring; notes FastAPI docs/static OpenAPI usage. |
| service/routes/marvel.py | Reimplements Marvel routes as FastAPI endpoints with OpenAPI descriptions. |
| service/routes/healthcheck.py | Reimplements healthcheck as FastAPI route with response model. |
| service/routes/dc.py | Reimplements DC routes as FastAPI endpoints with OpenAPI descriptions. |
| service/routes/init.py | Updates route registration to include_router(...). |
| service/models.py | Adds Pydantic models for request/response bodies. |
| service/config.py | Adds pydantic-settings Settings + cached accessor. |
| service/init.py | Replaces Flask factory with FastAPI factory, static mount, and exception handler. |
| requirements.txt | Removes pinned requirements in favor of pyproject.toml deps. |
| pyvenv.cfg | Removes checked-in virtualenv configuration file. |
| pyproject.toml | Adds project metadata, dependencies, dev tools, and pytest/coverage config. |
| main.py | Switches CLI entry to uvicorn.run(...) using Settings. |
| lgtm.yml | Removes legacy LGTM configuration. |
| config/default.example.cfg | Updates example environment config for new settings approach. |
| README.md | Updates documentation for FastAPI, uv-based setup, Make targets, and port changes. |
| Makefile | Adds uv-based install/run/test/lint/format/docker commands. |
| Dockerfile | Adds container build using uv, non-root user, and healthcheck. |
| .gitignore | Updates ignores for .venv and .env/config/.env. |
| .github/workflows/codeql-analysis.yml | Modernizes CodeQL action versions/permissions and adds main branch support. |
| .github/workflows/codacy-analysis.yml | Modernizes Codacy workflow and SARIF upload handling. |
| .flaskenv | Removes Flask CLI environment file. |
| .dockerignore | Adds docker ignore rules for venvs, tests, secrets, and build artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…raphics/simple-superhero-service-python into feature/flask-to-fastapi-upgrade
There was a problem hiding this comment.
Pull request overview
This PR migrates the service from Flask to FastAPI, modernizes packaging/build tooling (uv + pyproject), and reworks the test suite and operational assets (Makefile/Docker/CI) to align with the new runtime.
Changes:
- Replaced Flask app factory + Blueprints with FastAPI app factory + routers (DC/Marvel/healthcheck) and Pydantic models.
- Migrated packaging from
setup.py/requirements.txttopyproject.toml(Hatch) and addeduv-based workflows + Make targets. - Reorganized/expanded unit tests under
unit/and converted client usage tofastapi.testclient.TestClient.
Reviewed changes
Copilot reviewed 35 out of 42 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| unit/test_marvel.py | Updates endpoint tests for FastAPI redirect behavior and response parsing. |
| unit/test_dc.py | Updates endpoint tests for FastAPI redirect behavior and response parsing. |
| unit/test_api.py | Adjusts API utility + help endpoint tests for FastAPI responses. |
| unit/conftest.py | Switches fixtures to FastAPI TestClient and new app factory. |
| unit/test_file.py | Adds unit coverage for ReadFile filtering/sorting behaviors. |
| unit/test_factory.py | Adds tests for FastAPI app factory, healthcheck, and settings caching. |
| unit/test_error.py | Adds tests for InvalidUsage error normalization. |
| unit/test_common.py | Adds tests for ServiceUtils helpers. |
| tests/test_factory.py | Removes old Flask-based tests. |
| tests/README.md | Documents tests/ as reserved for integration tests via submodule. |
| tests/.this_is_a_git_submodule | Marks tests/ as a submodule placeholder. |
| service/init.py | Replaces Flask app factory with FastAPI factory + exception handler + static mount. |
| service/config.py | Introduces Pydantic Settings (get_settings) with .env support. |
| service/models.py | Adds Pydantic models for responses and POST request bodies. |
| service/routes/init.py | Updates route registration to FastAPI router inclusion. |
| service/routes/healthcheck.py | Replaces Flask healthcheck with typed FastAPI endpoint. |
| service/routes/marvel.py | Rewrites Marvel routing to FastAPI with query parsing + POST body support. |
| service/routes/dc.py | Rewrites DC routing to FastAPI with query parsing + POST body support. |
| service/routes/swagger.py | Removes Flask swagger blueprint (FastAPI docs used instead). |
| service/utils/api.py | Refactors config normalization, adds typing, updates docs. |
| service/utils/common.py | Adds typing and minor behavior refactor in helpers. |
| service/utils/error.py | Refines InvalidUsage typing/docstrings and dict conversion. |
| service/utils/file.py | Adds typing and refactors i18n-aware sorting into a helper. |
| service/settings.py | Removes Flask-era dotenv loader module. |
| main.py | Switches CLI startup from Flask to uvicorn with settings. |
| pyproject.toml | Adds project metadata, dependencies, pytest/coverage config, and Bandit config. |
| uv.lock | Adds locked dependency set for uv. |
| requirements.txt | Removes legacy pip requirements pin file. |
| setup.py | Removes legacy setuptools packaging. |
| setup.cfg | Removes legacy pytest/coverage config. |
| Makefile | Adds uv-based install, run, lint/format, test, coverage, docker targets. |
| Dockerfile | Adds container build using uv sync and uvicorn runtime conventions. |
| .dockerignore | Adds ignore rules for container context (incl. secrets/certs/tests). |
| .gitignore | Updates ignores for .venv and env files. |
| config/default.example.cfg | Updates example config format to match .env usage. |
| README.md | Updates docs for FastAPI/uvicorn/uv and new ports/commands. |
| .remarkrc.yml | Adds remark-lint configuration. |
| .github/workflows/codeql-analysis.yml | Updates CodeQL workflow to current actions versions and main/master branches. |
| .github/workflows/codacy-analysis.yml | Updates Codacy action + adds SARIF splitting and submodule checkout. |
| .flaskenv | Removes Flask CLI environment file. |
| pyvenv.cfg | Removes committed virtualenv metadata. |
| lgtm.yml | Removes LGTM configuration file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@morgangraphics I've opened a new pull request, #20, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@morgangraphics I've opened a new pull request, #21, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
* Initial plan * Fix POST description in dc.py to use 'characters' instead of 'character' Co-authored-by: morgangraphics <607594+morgangraphics@users.noreply.github.com> Agent-Logs-Url: https://github.com/morgangraphics/simple-superhero-service-python/sessions/adfde50c-baa8-4475-92be-57a754fa9768 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: morgangraphics <607594+morgangraphics@users.noreply.github.com>
| except OSError: | ||
| pass | ||
| @app.exception_handler(InvalidUsage) | ||
| async def handle_invalid_usage(request: Request, error: InvalidUsage) -> JSONResponse: |
Check notice
Code scanning / Prospector (reported by Codacy)
Unused argument 'request' (unused-argument)
service/routes/__init__.py
Outdated
| app.include_router(bp_dc) | ||
| app.include_router(bp_hc) | ||
| app.include_router(bp_marvel) | ||
|
|
Check notice
Code scanning / Prospector (reported by Codacy)
Trailing newlines (trailing-newlines)
| description=_BASE_DESCRIPTION, | ||
| ) | ||
| def dc_get_base( | ||
| characters: Annotated[Optional[str], Query(description="Character(s) to search for as a string value (e.g. a single name or a comma-separated list).")] = None, |
Check notice
Code scanning / Prospector (reported by Codacy)
line too long (163 > 159 characters) (E501)
| h: Annotated[Optional[str], Query(description="Headers to display. Either a string or Array of strings")] = None, | ||
| help: Annotated[Optional[str], Query(description=f"List available options. {_TF_TEXT}")] = None, | ||
| limit: Annotated[Optional[str], Query(description="Limit result set. '0' for no limit")] = None, | ||
| nulls: Annotated[Optional[str], Query(description="Sort null values either 'first' or 'last' in the sort order. Accepted values: 'first', 'last'.")] = None, |
Check notice
Code scanning / Prospector (reported by Codacy)
line too long (160 > 159 characters) (E501)
service/routes/healthcheck.py
Outdated
| def healthcheck() -> HealthResponse: | ||
| """Simple health check endpoint.""" | ||
| return HealthResponse(status="Ok") | ||
|
|
Check notice
Code scanning / Prospector (reported by Codacy)
Trailing newlines (trailing-newlines)
|
|
||
| # Sorting None must also survive sort direction (asc|desc) i.e. reverse=True|False | ||
| if self.config.get("nulls") == "first" and not self.config.get("prune"): | ||
| if not sort_dir: |
Check notice
Code scanning / Prospector (reported by Codacy)
Unnecessary "else" after "return", remove the "else" and de-indent the code inside it (no-else-return)
service/utils/file.py
Outdated
| else: | ||
| return (itm is None, itm != "", itm) | ||
| else: | ||
| if not sort_dir: |
Check notice
Code scanning / Prospector (reported by Codacy)
Unnecessary "else" after "return", remove the "else" and de-indent the code inside it (no-else-return)
unit/conftest.py
Outdated
| if request is not None and data.get(request.param): | ||
| val = data[request.param] | ||
| return val | ||
|
|
Check notice
Code scanning / Prospector (reported by Codacy)
Trailing newlines (trailing-newlines)
| assert response.status_code == 200 | ||
| data = response.json() | ||
| assert isinstance(data, list) | ||
|
|
Check notice
Code scanning / Prospector (reported by Codacy)
Trailing newlines (trailing-newlines)
| s1 = get_settings() | ||
| s2 = get_settings() | ||
| assert s1 is s2 | ||
|
|
Check notice
Code scanning / Prospector (reported by Codacy)
Trailing newlines (trailing-newlines)
FastAPI
uv
dependency updates
Makefile
Dockerfile
tests,
and more