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 support for changeable configurations #12

Open
smhmayboudi opened this issue Feb 9, 2024 · 6 comments
Open

Add a support for changeable configurations #12

smhmayboudi opened this issue Feb 9, 2024 · 6 comments

Comments

@smhmayboudi
Copy link

Thanks in advance for your hard work. I would like to suggest adding a feature related to configuration which is affected by the environment at runtime. In another world, there are some configurations which should be observed by the app and act due to changes. It may sound like flags, however it may be a good idea.

@holzensp
Copy link
Contributor

holzensp commented Feb 9, 2024

Could you expand on this? Maybe you can write up a basic design doc. As-is, this is too open-ended to know what "done" looks like.

@smhmayboudi
Copy link
Author

@holzensp do you have any template for the design doc?

The Imagination Feature

There is a hook which accepts a function as input. When the value of a config is changed the system calls that function (AFTER HOOK). This hook can be general for all changes related to a config. In this case, we should provide which field of config is changing or the previous state and next state of config to the function, so in the future the user or client of the package will decide what behaviour should be done. There is another solution, the hook function is provided for every config field with the previous function inputs. Same way we can have BEFORE HOOK as well.

What is the Usage?

I have a config address for connecting to a Redis server like redis-cluster-1:6379. Due to some changes in my structure, The address should be changed to redis-cluster-2:6379 when the service is up and running. With the current solution, I should restart the service. With this feature, I will introduce a function inside the service to behave to changes made at runtime for the config, so without restarting the service the new calls go to redis-cluster-2:6379 in this scenario.

@smhmayboudi
Copy link
Author

apple.pkl

beforeHook = beforeHookFN
afterHook = afterHookFN
name = "Apple Config Sample"

Generated GO or Inside the PKL-GO

Set(currentConfig, newConfig any) {
  beforeHook(currentConfig, newConfig)
  newConfig2 := newConfig
  afterHook(currentConfig, newConfig2)
  return newConfig2
}

func beforeHook(fn func(any, any), current, next any) {
  fn(current, next)
}

func afterHook(fn func(any, any), previous, current any) {
  fn(previous, current)
}

Usage

func beforeHookFN(current, next myAppleConfig) {
  // custom validation
  // propagate the config (next)
  // modify the config (next)
}

func afterHookFN(previous, current myAppleConfig) {
  // propagate the config (current)
  // modify the config (current)
}

@holzensp
Copy link
Contributor

holzensp commented Mar 12, 2024

Ah, change notifications... Isn't this something entirely for the host language (Go, in this case)? What is the expected behaviour when I change your apple.pkl to have

beforeHook = someFunctionThatDoesNotExist

?

The Pkl code won't know about the Go code you provide, so there's no way to check that.

We've had some discussions about adding an update facility to the Pkl library/server and letting that signal its consumers. From your suggestions here, I'm reading that you're only looking for course-grain notification; if anything in the config changes, reload the config and signal that changes have happened. Other things we've been thinking about is whether there's a use for fine-grain notification (per-property, essentially).

@smhmayboudi
Copy link
Author

when we use pkl-gen-go to generate the go code, the someFunctionThatDoesNotExist function definition will be available to the developer which should be implemented like grpc-middleware server part which should be implemented for every endpoint. your definition is correct, and I am looking for changes notification.

@holzensp
Copy link
Contributor

holzensp commented Apr 3, 2024

Sorry, missed the notification for your response :(

In this case, isn't it simpler to just watch the relevant Pkl file from the Go side and re-load it when a change is signalled? That "will be available" sounds a little magical. Do you mean pkl-gen-go should generate a stub that the user fills out? If it is, then I don't understand what beforeHook = beforeHookFN does in your Pkl file. I'm afraid you're going to have to describe a full step-by-step of what exists where and what creates what, if you want me to follow along with this (sorry; thick sometimes... or from a different context with different default assumptions).

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

No branches or pull requests

2 participants