Azure / data-api-builder

Data API builder provides modern REST and GraphQL endpoints to your Azure Databases and on-prem stores.
https://aka.ms/dab/docs
MIT License
917 stars 187 forks source link

[RFC]: Hot reload dab-config.json #1829

Open JerryNixon opened 1 year ago

JerryNixon commented 1 year ago

What happened?

For reference: #67

Hot reload

The idea of hot reload allows the direct manipulation of the dab-config.json file without requiring dab start.

Use cases

  1. mode=development allow a simpler workflow so developers can tweak the configuration file.
  2. mode=development allow administrators to modify minimum-log-level to diagnose problems.

Canges

Config: runtime.hot-reload-enabled=true|false default true.

When set to true, depending on mode the hot reload behavior will respond to changes in the dab-config.json file.

CLI: dab start --hot-reload-enabled true

When set to true, depending on mode the hot reload behavior will respond to changes in the dab-config.json file. If the same property is set to a conflicting value in the config file, the CLI value overrides it.

Config: runtime.minimal-logging-level=information|enum default based on mode.

Currently only available as an argument in dab start this allows in-memory modification through hot reload. mode=Production is the primary use case, allowing increased (or decreased) logging during troubleshooting.

Order of precedence In terms of precedence of value, the last thing that sets the log level wins.

  1. Default value is based on mode
    1. When mode=Production then Information
    2. When mode=Development then Debug
  2. When present in config then ignore the mode-based default.
  3. When present in the CLI then ignore the `config setting, except...
  4. When minimal-logging-level is set by hot reload then ignore current in-mem setting for hot reload value.

Behavior

config.mode config.hot-reload Behavior
Development True Auto-restart the engine with each/any config file save.
Production True React only to log level setting (or anything else we add later).
-any- False Nothing at all.
  1. For production it is important that the engine hot reload and not restarts
  2. For development it is not important that the engine hot reload and just restarts

Some questions

?. Should runtime.hot-reload-enabled also be hot reloadable in production? ?. Can we/should we implement the production version of this first? ?. Is "restart the engine" too drastic? Is there a simpler way to do the development version?

Version

Future

What database are you using?

Azure SQL

What hosting model are you using?

Local (including CLI)

Which API approach are you accessing DAB through?

REST, GraphQL

Relevant log output

No response

Code of Conduct

seantleonard commented 1 year ago

The following behavior is not really hot-reload. While probably way easier to implement, this just saves a dev the few seconds from having to hit ctrl + c and run dab start

For development it is not important that the engine hot reload and just restarts

aaronburtle commented 1 year ago

The following behavior is not really hot-reload. While probably way easier to implement, this just saves a dev the few seconds from having to hit ctrl + c and run dab start

For development it is not important that the engine hot reload and just restarts

I agree, and am wondering, aside from ease of implementation, if there is some benefit to restarting the engine this way, since you could do this yourself so easily.

Aniruddh25 commented 1 year ago

?. Is "restart the engine" too drastic? Is there a simpler way to do the development version?

Yes, restart engine seems unnecessary. That is NOT same as hot reload, why don't we keep it simple - just hot reload(refresh the engine only for the properties the mode supports the refresh?

Aniruddh25 commented 1 year ago

?. Can we/should we implement the production version of this first?

It was agreed upon previously that development version would be implemented first. If the requirement is now changing, we will NOT undo what we have implemented already for the development version but consider pivoting towards completing the production scenario first in future after finishing off whatever is in progress for development

Aniruddh25 commented 1 year ago

?. Should runtime.hot-reload-enabled also be hot reloadable in production?

Since we are only enabling hot reload of the loglevel in production, I think it should be fine to hot reload the runtime.hot-reload-enabled option itself, since even after enabling this - only loglevel can be reloaded.