configurable execution result store

[haileyok]

right now, there isn't a good way to configure or disable the execution result store. the only way i found was to use register_execution_result_store() in register_plugins but explicitly create and return a noop ExecutionResultStore. there's a misleading env var right now in the docker-compose.yml called OSPREY_EXECUTION_RESULT_STORAGE_BACKEND that doesn't seem to do anything as well. this results in the worker failing to process events as the requests to the minio store simply fail.

here, i took the similar route that exists for configuring the input stream

• create a new ExecutionResultStorageBackendType enum with Bigtable, GCS, Minio, or "plugin" as options, as well as a simple "None" option
• create a "chooser" that returns the supplied backend type
• update the sink register to use the new chooser rather than directly calling bootstrap_execution_result_store

[haileyok]

should none be the default, or should it be minio like the current behavior would result in?

[ayubun]

i think none, because minio is extra setup cost;

with that being said, i think we can have minio setup in the example plugins (?) we talked about moving it there during our sync today

[haileyok]

ya for now i left it here

osprey/osprey_worker/src/osprey/worker/_stdlibplugin/storage_register.py

Line 7 in 701f3d5

def register_execution_result_store(config: Config) -> ExecutionResultStore:

seems like a fine thing tho to move to examples, i can clean that up if ppl have opinions on what to do with it

[haileyok]

afaict, the ones i added here are the only ones that exist

[haileyok]

afaict, it makes sense to raise here. the codepaths i see calling bootstrap_execution_result_storage_service are all in the ui api or in one of the cli tools, and they probably should be raising an error when there isn't a configured backend.

[haileyok]

atp i only tested that this works with "none" as an option, i'll run with minio in a bit to make sure that still works

[ayubun]

i think none, because minio is extra setup cost;

with that being said, i think we can have minio setup in the example plugins (?) we talked about moving it there during our sync today

[julietshen]

hi @haileyok ! Is this PR ready for review?

[haileyok]

@julietshen it used to be, though I'm not sure what all has changed since I opened the PR...i do think its still needed though if you're not using minio iirc (i use minio for my own instance so haven't tried recently). i fixed merge conflicts i think

[julietshen]

@EXBreder I think this change just makes it so users CAN use something other than minio, do you mind giving it a review?

[haileyok]

fyi @cmttt idk if this affects how you're doing things but just flagging incase it does