-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Description
The Store class is too broad. It would be nice to see it split up to separate concerns, and avoid unsupported(...) default methods.
Intuitively, we have stores than just read and write files, and store that also can do builds. The latter would have primary and secondary systems, supportedFeatures, etc. etc. in the Config class.
But it's good to think about this distinction in the context of #5025 (comment), making buildPaths a top level method that works with a variety of store for different purposes.
Now buildPaths as a top-level method is not perfect because sometimes we do want to be able to let a remote store schedule too. E.g. when there is a bunch of remote builders, many people want to send jobs to a single central scheduler and let it dispatch jobs. So we don't want to just loose the ability of today's buildPaths implementation in RemoteStore and LegacySSHStore.
That means maybe it stays a method in some class (StoreThatCanSchedule?) after all? Or is StoreThatCanSchedule really the same as StoreThatCanBuild in practice? Would LocalStore need to implement this method, or should we just downcast and use the freestanding buildPaths function if it succeeds?
I don't know the answers to these; there is a lot to think through. Perhaps there is another easier split we can start with first, to start untangling things and improve our understanding?