Begin modularizing config options in a more modern way
ClosedPublic
Actions

Authored by epriestley on Jun 26 2017, 3:52 PM.

Details

Reviewers

chad
amckinley

Maniphest Tasks

T12845: Convert remaining ancient configuration type classes to modern modular type classes

Commits

rP03b6bdde1931: Begin modularizing config options in a more modern way

Summary

Ref T12845. Config options are "modular", but the modularity is very old, half-implemented, and doesn't use modern patterns.

Half the types are hard-coded, while half the types are semi-modular but in a weird hacky way where you prefix the type with custom:....

The actual API is also weird and requires types to return a lot of array($stuff, $thing, $other_thing, $more_stuff) sorts of tuples.

Instead:

Add a new replacement layer which uses modern modularity patterns and overrides the older stuff if available, so we can migrate things one at a time.
New layer uses a more modern API -- no return array($thing, $other_thing, ...), and more modern building blocks (like AphrontHTTPParameterType).
New layer allows custom types to be deleted, which will ultimately let us deal with T12845.

Then, convert the 'int' type to use the new layer.

Test Plan

Set, edited, tried-to-change-in-an-invalid-way, and deleted an 'int' option from the web UI.
Same from the CLI.
Edited config.json to have an invalid value, verified that the error was detected and config was repaired.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.Jun 26 2017, 3:52 PM

Harbormaster completed remote builds in B17572: Diff 43681.Jun 26 2017, 3:52 PM

Harbormaster completed remote builds in B17572: Diff 43681.

Edited config.json to have an invalid value, verified that the error was detected and config was repaired.

This only happens if you go back to the scene of the crime and attempt to edit the config again after making invalid config.json changes, right?

This revision is now accepted and ready to land.Jun 26 2017, 4:33 PM

This only happens if you go back to the scene of the crime and attempt to edit the config again after making invalid config.json changes, right?

If you restart Apache (or whatever your webserver is) or visit the "Setup Issues" page explicitly, that will also trigger it.

We could do something like hash local config files and re-validate them if they change, but it doesn't seem to be causing too many issues today, since you have to go manually mess with the file or manually mess with the database to create a problem in most cases. Usually, users hit this because they have something which used to be valid in local.json but no longer is after an upgrade (since validation got stricter), but they restart Apache as part of the process so the setup checks catch the issue.