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

Add a way for JSON-RPC V2 plugins to get settings on initialization/restart #3090

Open
cibere opened this issue Nov 22, 2024 · 10 comments
Open
Labels
enhancement New feature or request

Comments

@cibere
Copy link
Contributor

cibere commented Nov 22, 2024

Is your feature request related to a problem? Please describe.

I'm writing a python library that uses the V2 json-rpc api, and it really annoys me that although there is an initialization request that would be perfect for plugins to create a cache, plugins have to wait until the first query request is received to start building their cache, resulting in long first queries. This is because currently, the plugin's settings only seem to be sent to the plugin on query requests.

Describe the solution you'd like

I'd be ok with either of the following solutions:

  1. Add a GetSettings method to the public Flow API, so that plugins can request the settings on initialization.
  2. Include the current settings to the plugin in the initialize method's parameters (and based on how #Reload Context #3081 gets resolved, making sure its in the restart method's parameters as well)

Describe alternatives you've considered

Having the plugin read the plugin's settings file on restart, but although this would probably work as a "solution", this issue seems like it should have a better fix.

Additional context

Add any other context or screenshots about the feature request here.

@cibere cibere added the enhancement New feature or request label Nov 22, 2024
@taooceros
Copy link
Member

How about we abandon the setting passed via query and completely adopt the request/response model via a GetSetting and SetSetting?

@taooceros
Copy link
Member

@Yusyuriv @Garulf what do you think?

@Yusyuriv
Copy link
Member

Yeah, I also thought that it's a little awkward. It probably made sense in API v1, when plugin's whole lifecycle was start-respond to query-end, but having GetSettings/SetSettings for v2 sounds useful. I think ideal would be:

  1. Pass settings to the initialize method.
  2. Keep passing settings to query method.
  3. Add GetSettings/SetSettings methods for v2 plugins.
  4. Not sure if it would be useful, but maybe allow defining another optional request for v2 plugins, something like settings_updated, so plugins can subscribe to real-time changes to settings?

@Garulf
Copy link
Member

Garulf commented Nov 23, 2024

@Yusyuriv @Garulf what do you think?

This makes the most sense.

@Yusyuriv
Copy link
Member

@Yusyuriv @Garulf what do you think?

This makes the most sense.

I think you quoted the wrong message?

@taooceros
Copy link
Member

but would then the GetSetting be a little bit duplicate.

@Yusyuriv
Copy link
Member

If you're talking about it being redundant when there are points 1 and 2: I thought that there might be a use case when the user haven't used the plugin for a while, but the plugin needs to update some data for example every 5 minutes. It needs to do so using the current settings that might have changed since the last time the user used the plugin.

If you're talking about point 4, then I agree, GetSettings might be unnecessary here.

@taooceros
Copy link
Member

I actually feel like point 4 is quite interesting as a proactor mode. Was a little bit trapped by application lately. I will write some impl after this week.

Or on the other hand if you would like to proceed feel free to do so.

@taooceros
Copy link
Member

taooceros commented Dec 17, 2024

@Yusyuriv how do you think we make it event driven. When setting is changed by user, we invoke a event, which fires the changed part to the plugin (this can be done directly via the jsonrpc library with a c# event). Still we can implement a getSetting api.

@Yusyuriv
Copy link
Member

@Yusyuriv how do you think we make it event driven. When setting is changed by user, we invoke a event, which fires the changed part to the plugin (this can be done directly via the jsonrpc library with a c# event). Still we can implement a getSetting api.

So the event will only send the changed parts, and if you need to get all of the current settings, you'd use GetSettings (for example in initialize)? Sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants