- 
                Notifications
    You must be signed in to change notification settings 
- Fork 7
feat: multi-tenant aware STAC API #528
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
Conversation
| ], | ||
| "subtype": "ellipsoidal", | ||
| }, | ||
| }, | 
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 had to remove this in order to get the tests to pass (failures are viewable here https://github.com/NASA-IMPACT/veda-backend/actions/runs/17807450210/job/50622755224) I believe these are not related to my changes since this is happening generally on other PRs (see: #529)
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 am pretty sure that failed action is the pgstac bug that we can fix with an upgrade #527.
| Many of the changes I made here was just reformatting 0a94081 into separate files, adding some tests, and extending the custom landing page logic | 
| return request.path_params["tenant"] | ||
| return None | ||
|  | ||
| async def get_tenant_collections( | 
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 think we should explore using an lru_cache to cache an array of distinct collection ids for a tenant. These can be injected in every stac endpoint including search as a parameter as collections=col1,col2,col3 including collections/ and will handle the item level filtering for us. This will require less customization of the stac api behavior.
| Collections belonging to tenant | ||
| """ | ||
| collections = await super().all_collections(request, **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.
Even if collection properties searches are not working for us in pgstac 0.9.6, I think we should still consider using a keyword free-text query (i.e. https://openveda.cloud/api/stac/collections?lfilter-lang=cql2-text&q="disasters-tenant")
Or we could consider adding a top level tenant field (maybe even prefixed dashboard:tenant?) instead of a nested property. We are already adding fields outside of the base stac collection fields like dashboard:is_periodic and are staying compliant. If a tenant field works for us we could consider a custom extension for all of the dashboard related fields.
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.
RE free text collection search. This works for me with a local docker compose but I created a collection with keywords to test in staging and it is not working as expected. It is possible that maybe there is an issue with the migration from pgstac v0.8.5 to v0.9.6 if the image for v0.9.6 works but not our migrated db :(
https://staging.openveda.cloud/api/stac/collections/z-test-keywords-hd-blackmarble-nightlights
https://staging.openveda.cloud/api/stac/collections?filter-lang=cql2-text&q="disasters-tenant"
EDIT (moved comment from wrong discussion)
| @anayeaye I'm having a bit of trouble with the idea of using keywords, seems less "official" when it comes to tenant management and doesn't seem like the typical intended use of keywords. | 
| @smohiudd @botanical I see what you are saying about using keywords for tenant filtering and agree that it probably doesn't make sense for this use case. Pros: 
 Cons: 
 | 
| I am aware the STAC browser is not loading and actively looking into it ! | 
…tion for skipping tenant processing
| Going to close this since we are going with other implementation #531 ! | 
Issue
#525
What?
The main changes between my recent commits and the original is that this PR:
fake-tenantsetproperties.tenantTesting?
I deployed and tested on the jtran-dev stack. I update two collections to belong to a tenant called
fake-tenant:campfire-lst-day-diffandcampfire-lst-night-diffSTAC Browser: https://jtran.delta-backend.com/
STAC Catalog: https://jtran.delta-backend.com/api/stac/
Collections: https://jtran.delta-backend.com/api/stac/collections
Tenant Collections: https://jtran.delta-backend.com/api/stac/fake-tenant/collections