Skip to content
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

feat: tipg-api #62

Merged
merged 5 commits into from
Aug 30, 2023
Merged

feat: tipg-api #62

merged 5 commits into from
Aug 30, 2023

Conversation

vincentsarago
Copy link
Member

@vincentsarago vincentsarago commented Aug 4, 2023

closes #51

Todo

  • Add docs (readme)

@vincentsarago vincentsarago marked this pull request as ready for review August 4, 2023 21:57
@vincentsarago vincentsarago marked this pull request as draft August 4, 2023 21:58
@emileten
Copy link
Contributor

@vincentsarago is the only TODO left to add a readme ? It looks like the rest is done to me, but I'm not 100% sure because I haven't worked with tipg yet.

@vincentsarago
Copy link
Member Author

@emileten yes, I'll try to pickup this PR tomorrow but if you want to have a look and check that I haven't done anything stupid feel free 🙏

@vincentsarago vincentsarago marked this pull request as ready for review August 24, 2023 08:50
@emileten
Copy link
Contributor

@vincentsarago I integrated this to our maap-eoapi stack, and it deployed successfully, however a get request to the resulting tipg HTTP API returns me Internal Server Error. Here is the lambda log :

[ERROR] Runtime.ImportModuleError: Unable to import module 'src.handler': No module named 'utils'
Traceback (most recent call last):

@emileten
Copy link
Contributor

solved here.

it's the same problem that was caused by #61 but there it was with the tiler.

Here, the problem was : we do from tipg.main import app at the top of the handler. Doing this import triggers a postgres settings creation, but the postgres environment variables are not yet loaded into the environment, it was happening after all imports. There are only two ways around this :

  1. load the postgres env vars before the imports (we don't want to have them in the lambda configuration directly, because it exposes secrets). That's what I did here.
  2. don't rely on from tipg.main import app and have a custom app instead. That's what @vincentsarago was suggesting we do here, and that's what we do for stac-fastapi (however I'm not sure the stac-fastapi provide an app layer that's reusable like tipg or titiler-pgstac).

Back when I added the titiler pgstac construct here I decided to reuse the standard app layer from the titiler package (titiler.main.app) nbecause I saw no reason to duplicate code.

Anyways, just like for the tiler bug, I chose (1) to solve this bug.

Copy link
Contributor

@emileten emileten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vincentsarago I think it's now good to go (see above), feel free to merge. A couple things to note.

  • Could you point me to an example running tipg api ? I am not familiar with the technology and I would like to have a look. Is the eoapi dev a good place to look at ?
  • Do you have examples of inserting data into the pgstac database that tipg can read ? I tried deploying this connected to the MAAP pgstac database but we only have STAC records there, so nothing that tipg could read, if I understand well.
  • We should add an option for a tipg custom domain name later, just like we did here for the stac-api, the stac-ingestor, and titiler-pgstac.

@vincentsarago
Copy link
Member Author

Could you point me to an example running tipg api ? I am not familiar with the technology and I would like to have a look. Is the eoapi dev a good place to look at ?

Yes, https://vector.eoapi.dev

Do you have examples of inserting data into the pgstac database that tipg can read ?

Not really, tipg is not meant to render data from pgstac but maybe ingested in other schemas. We can render pgstac tables directly but the schema is not optimized for this use case. The idea of tipg is to be able to store vector dataset that could be used in a frontend application in addition to STAC metadata and Raster tiles

We should add an option for a tipg custom domain name later, just like we did #63

I think we should do it in the PR as well 🙏

Thank @emileten for this nice review and fixes 🙌

@emileten
Copy link
Contributor

I think we should do it in the PR as well 🙏

Oh yes let's do it here.

Not really, tipg is not meant to render data from pgstac but maybe ingested in other schemas. We can render pgstac > tables directly but the schema is not optimized for this use case. The idea of tipg is to be able to store vector dataset > that could be used in a frontend application in addition to STAC metadata and Raster tiles

Ok maybe I need to rephrase my question @vincentsarago and it also shows the limit of my knowledge about databases. So we're connecting tipg to the same RDS instance as titiler-pgstac and stac-api, here for example. Is tipg going to read from another database in the same instance ? Do you have examples of ingesting data into an RDS instance that is then read by tipg?

@vincentsarago
Copy link
Member Author

Ok maybe I need to rephrase my question @vincentsarago and it also shows the limit of my knowledge about databases. So we're connecting tipg to the same RDS instance as titiler-pgstac and stac-api, here for example. Is tipg going to read from another database in the same instance ? Do you have examples of ingesting data into an RDS instance that is then read by tipg?

Yeah, by default tipg will connect to the public schema (while stac-api will connect to the pgstac schema).

Do you have examples of ingesting data into an RDS instance that is then read by tipg?

Not really, you can do psql -f countries.sql using https://github.com/developmentseed/tipg/blob/main/data/countries.sql but you have to have direct access to the database

@emileten emileten merged commit 24faa85 into main Aug 30, 2023
@emileten emileten deleted the FeatTipgAPI branch August 30, 2023 08:45
github-actions bot pushed a commit that referenced this pull request Aug 30, 2023
# [5.2.0](v5.1.0...v5.2.0) (2023-08-30)

### Features

* tipg-api ([#62](#62)) ([24faa85](24faa85))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a vector application construct
2 participants