-
Notifications
You must be signed in to change notification settings - Fork 30
update titiler/titiler-pgstac #252
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: main
Are you sure you want to change the base?
update titiler/titiler-pgstac #252
Conversation
return result | ||
|
||
|
||
# /tiles endpoint compat with titiler<0.15, Optional `tileMatrixSetId` |
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've added this because I thought we needed default
WebMercatorQuad tilematrixset back.
We can remove if not needed
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.
Let's keep it, I see in our logs that we do have callers that request tiles without a TMS in the path.
For others, you can verify with this query in pc-api-appinsights
:
ProductionRequests
| where name == "GET" and service == "tiler"
| where not(url has "/preview.png")
| where url has "item/tiles" and not(url has "tiles/WebMercatorQuad")
def tile_routes( # type: ignore | ||
request: Request, | ||
search_id=Depends(pgstac_mosaic_factory.path_dependency), | ||
tile=Depends(TmsTileParams), |
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.
TmsTileParams
is not used anymore in titiler-pgstac so it might disappear 🙈
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.
Not a problem!
The CI/CD pipeline for this repo is functional. Once you pass the pipeline and merge this into main, it will be deployed and available at |
I have edited the settings to allow your commits to run CI/CD automatically. |
7eab044
to
f6ee228
Compare
@ghidalgo3 could you help me debug the azurite error message 🙏 |
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.
@vincentsarago do you know why all of these requirement versions got bumped? I created a branch off of your branch to work on telemetry and when I ran ./scripts/generate-requirements
the pinned versions all got bumped back to match the main branch 🤔
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 usually delete the previous files and re-generate from scratch (I think I had issues before trying to update the files in place)
This PR does:
TODO