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

Rust Hydration #201

Merged
merged 13 commits into from
Sep 19, 2023
Merged

Rust Hydration #201

merged 13 commits into from
Sep 19, 2023

Conversation

bitner
Copy link
Collaborator

@bitner bitner commented Aug 21, 2023

  • update pydantic requirement to ~=2.0
  • update docker and ci workflows to build binary wheels for rust additions to pypgstac
  • split docker into database service and python/rust container
  • Modify scripts to auto-generate unreleased migration
  • Add pre commit tasks to generate migration and to rebuild and compile pypgstac with maturin for rust
  • Add private jsonb column to items and collections table to hold private metadata that should not be returned as part of a stac item
  • Add generated columns to collections with the bounding box as a geometry and the datetime and end_datetime from the extents (this is to help with forthcoming work on collections search)
  • Add PLRust to the Docker postgres image for forthcoming work to add optional PLRust functions for expensive json manipulation (including hydration)

@bitner bitner mentioned this pull request Aug 21, 2023
@bitner bitner marked this pull request as ready for review August 21, 2023 20:59
@bitner bitner requested a review from mmcfarland August 21, 2023 20:59
Copy link
Contributor

@kylebarron kylebarron left a comment

Choose a reason for hiding this comment

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

I'm not very familiar with the postgres internals of pgstac, but the Rust code and Rust CI changes look good

src/pypgstac/Cargo.toml Outdated Show resolved Hide resolved
src/pypgstac/pyproject.toml Show resolved Hide resolved
src/pypgstac/src/lib.rs Outdated Show resolved Hide resolved
src/pypgstac/src/lib.rs Outdated Show resolved Hide resolved
@gadomski gadomski self-requested a review August 30, 2023 19:13
@vincentsarago
Copy link
Member

@bitner, This PR is blocking some other PR and projects. What do you think about splitting this into multiple PR (e.g #197) so we can move forward.

@bitner
Copy link
Collaborator Author

bitner commented Sep 15, 2023

Main blocker is me being out of the office. I'm hoping to get everything merged on Monday. If i hit any road blocks I'll split things up

@bitner
Copy link
Collaborator Author

bitner commented Sep 18, 2023

@kylebarron @gadomski I updated to use pyo3 rather than serde using sample code from @gadomski. As we discussed last week, my main goal here is to get the infrastructure into place for incorporating rust with hopefully a bit of a win on hydration performance, but more with an eye towards moving much of the json parsing as well as things like injecting links into rust in the future.

scripts/pgstacenv Outdated Show resolved Hide resolved
src/pypgstac/Cargo.lock Outdated Show resolved Hide resolved
src/pypgstac/Cargo.toml Outdated Show resolved Hide resolved
src/pypgstac/Cargo.toml Show resolved Hide resolved
src/pypgstac/Cargo.toml Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
src/pypgstac/Cargo.toml Show resolved Hide resolved
src/pypgstac/src/lib.rs Outdated Show resolved Hide resolved
@gadomski gadomski self-requested a review September 19, 2023 13:47
This fixes a permissions error when running the test scripts as non-root.
Copy link
Member

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

@bitner I made a couple of touchups to the Rust code and the pypgstac Dockerfile. LGTM for a first-pass, :shipit:.

@bitner
Copy link
Collaborator Author

bitner commented Sep 19, 2023

Thanks @gadomski and @kylebarron for all the review / code! I'm going to merge this now, but not going to cut a release until I do some heavy benchmarking with PC.

@bitner bitner merged commit 90d4364 into main Sep 19, 2023
15 checks passed
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.

4 participants