Hi Pengcheng, Looks like a solid improvement, definitely helping people using their own state store.
I have a few comments: 1. Background knowledge should explain what is a state storage 2. Move problem description from Background Knowledge to Motivation. I'm quoting the template to understand what should be included in the Background knowledge section: <!-- Describes all the knowledge you need to know in order to understand all the other sections in this PIP * Give a high level explanation on all concepts you will be using throughout this document. For example, if you want to talk about Persistent Subscriptions, explain briefly (1 paragraph) what this is. If you're going to talk about Transaction Buffer, explain briefly what this is. If you're going to change something specific, then go into more detail about it and how it works. * Provide links where possible if a person wants to dig deeper into the background information. DON'T * Do not include links *instead* explanation. Do provide links for further explanation. EXAMPLES * See [PIP-248](https://github.com/apache/pulsar/issues/19601), Background section to get an understanding on how you add the background knowledge needed. (They also included the motivation there, but ignore it as we place that in Motivation section explicitly) --> 3. `WorkerConfig` - explain briefly what is Worker and how it differs from Broker. Should be in background knowledge section. 4. Background knowledge should explain briefly what is a runtime and runtime factory. 5. Add a new cli argument to JavaInstanceStarter and LocalRunner so > process&k8s runtime can pass state related config to them Today all config goes through arguments and not a file? 6. `--stateStorageConfig` What format is the expected value? 7. `functions local run` What is this? 8. Are you keeping `stateStorageServiceUrl`? Maybe people rely on it? 9. Don't forget to include link to discussion thread using Apache Pony Mail On Mon, May 29, 2023 at 10:44 AM Rui Fu <r...@apache.org> wrote: > Hi Pengcheng, > > Thanks for bringing this up, the PIP lgtm, +1. > > Best, > > Rui Fu > On May 29, 2023 at 13:52 +0800, Enrico Olivelli <eolive...@gmail.com>, > wrote: > > Looks good > > +1 > > > > Enrico > > > > Il Lun 29 Mag 2023, 04:47 Pengcheng Jiang > > <pengcheng.ji...@streamnative.io.invalid> ha scritto: > > > > > Dear Pulsar community, > > > > > > I created a pip to make pulsar functions' `StateStoreProvider` > configurable > > > with custom configurations: > https://github.com/apache/pulsar/issues/20419 > > > > > > Any feedback and suggestions are welcome > > > > > > Sincerely > > > Pengcheng Jiang > > > >
