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

Support multiple commands #290

Merged
merged 52 commits into from
Mar 23, 2017
Merged

Conversation

tgross
Copy link
Contributor

@tgross tgross commented Mar 7, 2017

This PR is a work in progress for supporting multiple "main" applications on a first-class basis. It involves a fairly major refactoring of the internals.

Supporting more than one "main" application and their various preStart behaviors implies they can depend on each other. Rather than making us responsible for creating a graph of these dependencies, all components will publish and subscribe to events on a top-level EventBus. Services are split into Services and HealthChecks, Coprocesses are treated as non-advertised Services, and Tasks are treated as repeating non-advertised Services. To do this in a way that doesn't require updating all our configuration syntax at the same time, I'm having to refactor the configuration parsing code as well to more strongly separate it from the app state.

Major TODOs:

  • update coprocess config to create Service for each task
  • update tasks config to create a Service for each task
  • update sensors to use the same EventBus (i.e. implement the Runner interface)
  • update the signal handlers to fire events on the EventBus
  • represent preStart, postStop, and preStop in terms of events
  • pull some more of the config parsing out of the core package and push it into config where it belongs
  • update all the subprocess execution points to take a Context so they can be cancelled by the events we hit in the central Run loop for each service/task/etc.
  • get all our integration tests passing again after all this work (I'm trying to make sure unit tests pass the whole way)

cc @misterbisson @geek @jasonpincin

@tgross
Copy link
Contributor Author

tgross commented Mar 7, 2017

(Still working on getting coprocess and task ported, but I think I'll have that by end of day tomorrow.)

@tgross
Copy link
Contributor Author

tgross commented Mar 8, 2017

Well perhaps a single day was ambitious... 😀

I've got the skeleton of work for how I'm splitting out the various Config structs from Runnables so that we can parse all the config for task and coprocess and create Services from those configs. I'm working to get everything compiling and unit tests passing but the overall concept should be understandable now.

@tgross
Copy link
Contributor Author

tgross commented Mar 10, 2017

I've got a majority of the big TODOs hit at this point. The signal handling needs to be reworked and I need to hit where I've stubbed out the preStart, preStop, etc. behaviors (that should be minor). Hopefully will wrap this PR up early next week.

@tgross
Copy link
Contributor Author

tgross commented Mar 23, 2017

Ok as of fd05aad I have all the tests passing again in Travis build 214283103. There's a tiny bit of cleanup to do to make sure I haven't left in any dumb debugging messages, clear out a couple commented-out bits, etc. and then I'll merge this.

This PR honestly isn't the cleanest thing in the world and merging in 3 weeks of work instead of a bunch of smaller commits sucks, but this will enable us to complete the rest of the v3 work as a series of smaller commits.

Note that I'm intentionally not updating documentation for the breaking config changes here, as I don't want the docs on Joyent.com to be updated yet (they're compiled automatically from master) until we complete the config language changes.

@tgross tgross changed the title [WIP] Support multiple commands Support multiple commands Mar 23, 2017
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

Successfully merging this pull request may close these issues.

2 participants