Skip to content

feat: add postgres result store#171

Merged
haileyok merged 2 commits intoroostorg:mainfrom
serendipty01:serendipty01/add-pg-execution-result-store
Mar 7, 2026
Merged

feat: add postgres result store#171
haileyok merged 2 commits intoroostorg:mainfrom
serendipty01:serendipty01/add-pg-execution-result-store

Conversation

@serendipty01
Copy link
Contributor

Added for ease in local development

Need to change below env to enable this
OSPREY_EXECUTION_RESULT_STORAGE_BACKEND=postgres

Note:
I haven't tested with druid setup
Also, I'm not deeply familiar with Python or this codebase, so let me know anything that needs to be changed or improved.

Tested with Hailey's fork
image
image

Copy link
Member

@ayubun ayubun left a comment

Choose a reason for hiding this comment

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

i like the pr, but i don't love some of the naming in it. i think we should change all of the existing names (perhaps in a different pr)

@EXBreder @haileyok maybe we should move discord to this store for localdev, since we already have a postgres locally for other elements of the ui & the bigtable simulator is annoying to use 😆

return execution_result_dict


class StoredExecutionResultPostgres(ExecutionResultStore):
Copy link
Member

Choose a reason for hiding this comment

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

i know you're just following conventions that have existed before you, but i'm not in love with this naming scheme. perhaps for a different pr, it could be renamed to something more descriptive:

Suggested change
class StoredExecutionResultPostgres(ExecutionResultStore):
class PostgresExecutionResultStore(BaseExecutionResultStore):

the term StoredExecutionResult implies that the class is a pydantic object / dataclass, but this is just an interface for the result store

Copy link
Collaborator

@haileyok haileyok left a comment

Choose a reason for hiding this comment

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

nice! im curious since you were testing w/ my fork and atproto data, were you handling the full firehose throughput? im curious what the perf on inserts was for you!

i pushed a changelog entry too for you, we need to put that in the pr template 😅

@haileyok haileyok merged commit 6bbc9a9 into roostorg:main Mar 7, 2026
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.

3 participants