-
Notifications
You must be signed in to change notification settings - Fork 2
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
Allow directory with template overrides #9
Conversation
@@ -41,7 +41,7 @@ async def features( | |||
limit: int = None, | |||
offset: int = None, | |||
**kwargs: Any, | |||
) -> Items: | |||
) -> FeatureCollection: |
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.
?
Items
is a FeatureCollection based model
tifeatures/tifeatures/model.py
Lines 114 to 134 in 28afbd8
class Items(FeatureCollection): | |
"""Items model | |
Ref: http://schemas.opengis.net/ogcapi/features/part1/1.0/openapi/schemas/featureCollectionGeoJSON.yaml | |
""" | |
id: str | |
title: Optional[str] | |
description: Optional[str] | |
keywords: Optional[List[str]] | |
features: List[Item] # type: ignore | |
links: Optional[List[Link]] | |
timeStamp: Optional[str] | |
numberMatched: Optional[int] | |
numberReturned: Optional[int] | |
class Config: | |
"""Link model configuration.""" | |
arbitrary_types_allowed = True |
(Gives better documentation)
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.
Mypy was complaining, i was just trying to get the tests to pass (same with everything in that last commit).
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.
👍 I'll check that 🙏
|
||
url = self.url_for(request, "items", collectionId=collection.id) | ||
if query_params: | ||
url += f"?{query_params}" | ||
url += f"?{QueryParams(**query_params)}" |
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 this related to the PR topic?
I'm fine with the change, but I worry that I did use this kind of style at multiple places 🤷♂️
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.
Again, mypy didn't like that query_params was a dict in some places and QueryParama elsewhere
def create_html_response( | ||
request: Request, data: str, template_name: str | ||
) -> HTMLResponse: | ||
) -> _TemplateResponse: |
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.
👍
settings = APISettings() | ||
|
||
|
||
class SearchPathTemplates(Jinja2Templates): |
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.
what about something a bit simpler:
template_dir = str(pathlib.Path(__file__).parent.joinpath("templates"))
templates = Jinja2Templates(directory=template_dir)
def create_template_response(name: str, context: dict, **kwargs: Any):
if settings.template_directory:
custom_templates = Jinja2Templates(directory=settings.template_directory)
try:
return custom_templates.TemplateResponse(name, context, **kwargs)
except: # TODO Add more precise exception handling
pass
return templates.TemplateResponse(name, context, **kwargs)
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.
The thing i like about extending SearchPathTemplates is that it allows us to use the native tooling in jinja and could handle several paths if anyone needed as well.
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.
👍 it seems you can do this without re-writing your own class encode/starlette#1214
Closing this PR out in favor of PR #11. |
Allow a directory with templates to override the default templates that are delivered with tifeatures.