Fastapi confserver no cache #1181
Conversation
# Conflicts: # curiefense/curieconf/server/curieconf/confserver/__init__.py
…/flask_to_fastapi
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Unit Test Statistics 1 files 1 suites 2m 53s ⏱️ results for commit d473c4a |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
E2E Test Statistics 1 files 1 suites 4m 0s ⏱️ Results for commit d473c4a. |
|
|
||
|
|
||
| ## Import all versions | ||
| from .v3 import api as api_v3 | ||
|
|
||
| app = Flask(__name__) | ||
| logging.basicConfig( | ||
| handlers=[logging.FileHandler("fastapi.log"), logging.StreamHandler()], |
There was a problem hiding this comment.
Do we need store logs in file? Usually it's enough to just stdout. Not sure
| format="[%(asctime)s] {%(pathname)s:%(lineno)d} %(levelname)s - %(message)s", | ||
| datefmt="%H:%M:%S", | ||
| ) | ||
| logger = logging.getLogger("filters-maxmind") |
| logging.basicConfig( | ||
| handlers=[logging.FileHandler("fastapi.log"), logging.StreamHandler()], | ||
| level=logging.INFO, | ||
| format="[%(asctime)s] {%(pathname)s:%(lineno)d} %(levelname)s - %(message)s", |
There was a problem hiding this comment.
asctime - print timestamp. k8s logs adds it's own timestamps into logs. How do we print logs in other curiefense services - with or without timestamps?
| # TODO - find replacements for got_request_exception and prometheus_flask_exporter | ||
| # if options.pdb: | ||
| # flask.got_request_exception.connect(drop_into_pdb) | ||
| # metrics = PrometheusMetrics(app) |
There was a problem hiding this comment.
If the replacement was found - let's remove commented strings from the code
| app.options = options.__dict__ | ||
| uvicorn.run(app, host=options.host, port=options.port) | ||
|
|
||
| # app.run(debug=options.debug, host=options.host, port=options.port) |
There was a problem hiding this comment.
No need to leave commented string if it's not functional and not a comment
| from curieconf.utils import cloud | ||
|
|
||
| # TODO: TEMP DEFINITIONS |
| from flask import Blueprint, request, current_app, abort, make_response | ||
| from flask_restx import Resource, Api, fields, marshal, reqparse | ||
| from curieconf import utils | ||
| # from curieconf import utils |
| import os | ||
|
|
||
| router = APIRouter(prefix="/api/v3") | ||
| options = {} | ||
| val = os.environ.get("CURIECONF_TRUSTED_USERNAME_HEADER", None) | ||
| if val: | ||
| options["trusted_username_header"] = val | ||
| val = os.environ.get("CURIECONF_TRUSTED_EMAIL_HEADER", None) | ||
| if val: | ||
| options["trusted_email_header"] = val | ||
|
|
||
| import requests | ||
| from jsonschema import validate | ||
| from pathlib import Path | ||
| import json | ||
|
|
There was a problem hiding this comment.
Let's keep an order in imports and code maybe?
- python and 3rd party modules
- empty line
- our modules
- 2 empty lines
- code
| exclude: typing.Any | ||
| key: anyTypeUnion | ||
| pairwith: typing.Any | ||
| tags: List[StrictStr] |
There was a problem hiding this comment.
This one and threshold was optional (no "required=True") and now it's not. What do we want here?
| version: Optional[StrictStr] | ||
| # TODO - dt_format="iso8601" | ||
| date: Optional[datetime.datetime] | ||
| # star_: Optional[List[typing.Any]] = Field(alias="*") |
| @router.get( | ||
| "/configs/", | ||
| tags=[Tags.congifs], | ||
| response_model=List[Meta], | ||
| response_model_exclude_unset=True, | ||
| ) | ||
| async def configs_get(request: Request): |
There was a problem hiding this comment.
If we had descriptions for fields and methods in the previous version - let's add this here too (for here and all the rest API routes below too)
| if x_fields.startswith(("[", "{", "(")): | ||
| x_fields = x_fields[1:-1] | ||
| x_fields = x_fields.replace(" ", "") | ||
| fields = x_fields.split(",") |
There was a problem hiding this comment.
Or we need to refactor it, or leave a good comment with in and out examples to understand what's going on here. And probably still comment like # TODO: refactor this in addition
| """Get all versions of a given configuration""" | ||
| res = request.app.backend.configs_list_versions(config) | ||
|
|
||
| if request.headers.get("X-fields", False): |
There was a problem hiding this comment.
We have more elegant way to get headers in FastAPI, take a look
There was a problem hiding this comment.
Also this will add it into swagger
| config: str, blob: str, version: str, request: Request | ||
| ): | ||
| """Create a new version for a blob from an old version""" | ||
| return request.app.backend.blobs_revert( |
There was a problem hiding this comment.
Is there a way in FastAPI to pass request.app as a dependency?
I suspect so, but not sure. If yes - we may make a lot of places more elegant
| if x_fields: | ||
| if x_fields.startswith(("[", "{", "(")): | ||
| x_fields = x_fields[1:-1] | ||
| x_fields = x_fields.replace(" ", "") | ||
| fields = x_fields.split(",") |
There was a problem hiding this comment.
I think we had similar function up there. Maybe it's different. If so - need to leave comment with in and out examples and TODO to refactor it
| if document not in models: | ||
| raise HTTPException(404, "document does not exist") | ||
| res = request.app.backend.documents_list_versions(config, document) | ||
| # res_filtered = [{key: r[key] for key in list(VersionLog.__fields__.keys())} for r in res] |
| @router.post("/db/{nsname}/", tags=[Tags.db]) | ||
| async def ns_resource_post(nsname: str, db: DB, request: Request): | ||
| """Create a non-existing namespace from data""" | ||
| _db = await request.json() |
There was a problem hiding this comment.
seems like db is not in use?
No description provided.