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

define bootstrap #22

Closed
plastikfan opened this issue May 7, 2024 · 7 comments · Fixed by #23 or #28
Closed

define bootstrap #22

plastikfan opened this issue May 7, 2024 · 7 comments · Fixed by #23 or #28
Assignees
Labels
feature New feature or request

Comments

@plastikfan
Copy link
Contributor

plastikfan commented May 7, 2024

Its hard to specify what the requirement is upfront, so they will emerge as we experiment on how to apply the correct patterns to get this right from day one, without making the mistakes of the past.

Actually, I find it useful to get my thoughts out into written form, so this issue will appear like a work journal tracking decisions made and their raison d'etres.

@plastikfan plastikfan added the feature New feature or request label May 7, 2024
@plastikfan plastikfan self-assigned this May 7, 2024
@plastikfan
Copy link
Contributor Author

can't define this properly until the user interface has been defined. In fact, the better way forward should be to start from the outside and then work inwards. Trying to specify the bootstrap at this point is starting from the middle, which is too difficult and doomed to failure.

@plastikfan
Copy link
Contributor Author

plastikfan commented May 9, 2024

At this point, what we have established is the following:

  • Each feature implmented as an internal plugin. That doesn't mean we have to support client defined plugins, its just a way of building the library constisting of different features which are decoupled from each other and from the kernel
  • We will use a message bus to help create a bootstrap that enables efficient start up and less coupled plugins
  • Each plugin will register a handler with the messaging broker inside an init func (this is considered ok because the init funcitonality will not impede testing ability). Usually I would avoid the init function like its the plague. But in this scenario, it provides the correct initialisation scheme without impeding our ability to writing effective unit tests
  • the Data member of the bus.Message, will contain the appropriate object (via an interface) that relates to the topic(s), the plugin subscribes
  • Since the registry is of internal concerns only, it should be moved to an internal package. This will necessitate moving the life cyle handler definitions to core, to avoid circular dependencies

We might need to move the broker instance into an internal package as it is an internal concern that the clinet should not be able to access.

@plastikfan
Copy link
Contributor Author

plastikfan commented May 11, 2024

It occurs to me that the core navigators should not contain any filtering capabilities. This is because one of the principle reasons for wanting to use rx, is its ability to create an observable with filtering. This model is much better than my first attempt in extendio. So if the client does not need extended features like filtering/hibernation then they can use Walk to perform a simple traversal. When the client needs these higher level fetaures, then they would use Run which internally creates an observable and can be configurable by the client.

In actual fact, the client can still implement a filtering capability when using Walk, by defining a ReadDirectory hook. The same principle applies to the sampling feature, which could be implemented directly inside the same hook.

But it seems like we have different features competing over the same hook. What if the client wants to sample, but they also want a hook? This would suggest that the life time hook could be processed by more than 1 plugin/callback, but we have no provision for this at the moment.

Sampling must be implemented as a ReadDirectory hook, that supplements the default hook. The sampling hook will use the filter defined in the options; but we could also have a separate filter specifically for sampling that overrides the default filter if defined; so the filter can be defined in more than 1 place in options.

@plastikfan
Copy link
Contributor Author

plastikfan commented May 14, 2024

trying to use the abstract factory pattern is difficult to apply for the navigator and actually doesn't seem to fit the problem in hand. The pattern as defined here assumes you have concrete things that need to be constructed, eg a chair of a sofa. This doesnt translate to the navigator. In our case we have 1 thing that needs to be constructed, but that 1 thing, the navigator has 2 properties:

  • extent: primary or resume session
  • sync: sequential or reactive

Mmm, having said that, it occurs to me that we do have multiple things; what we want to create are

  • driver
  • session

For the purposes of discussion, let's group these together into a single entity called the sentinel.

The sentinel can be any 1 of four instances:

  • primary-sequential
  • primary-reactive
  • resume-sequential
  • resume-reactive

Actually, the builder seems to be a better fit, so let's explore here. The first probem I have with the navigator is that it is a single entity with 2 characteristics as opposed to a single overall object with child enties as in the example; ie a house, with walls, doors etc. You can constuct a door or window in their own right and have them as members of the house. But this doesnt work for the navigator. You can't create an extent or a sync in their own right, they are merely characteristics of the navigator, so it is still difficult to apply the builder pattern, unless I can make a different leap somehow.

Maybe I need to look at some of the behavioural patterns, but then we are outside of creational patterns. What we want is a creational pattern incorporating a behaviour pattern; so we need to start by analysing the behaviour patterns, finding one which matches and then incorporate that into a builder pattern. But looking at the list, I am already familiar with all those patterns, and none of them fit.

@plastikfan
Copy link
Contributor Author

plastikfan commented May 14, 2024

We need to ensure our build process fits into the different ways the client interacts with traverse which are identified in the following snippets:

Primary Walk:

	_, err := traverse.Walk().Primary(
		"/root-path",
		func(_ *core.Node) error {
			return nil
		},
	).Configure(
		pref.WithSubscription(enums.SubscribeFiles),
	).Navigate()

Resume Walk:

	_, err := traverse.Walk().Resume(
		"/from-restore-path",
		enums.ResumeStrategyFastward,
	).Configure().Navigate()

Primary Run:

	_, err := traverse.Run().Primary(
		"/root-path",
		func(_ *core.Node) error {
			return nil
		},
	).Configure(
		pref.WithSubscription(enums.SubscribeFiles),
		pref.WithContext(ctx),
	).Navigate()

Resume Run:

	_, err := traverse.Run().Resume(
		"/from-restore-path",
		enums.ResumeStrategySpawn,
	).Configure().Navigate()

Since Walk and Run are functions, we create the build director inside of them so it can coordinate the construction process. The sentinel should not contain any reference to build objects as the build directory must cease to exist anywhere after construction. It is fine to pass in any other build inputs to Walk and Run, but they should not be retained in the sentinel or navigator.

So what we have at the moment is, the sentinel artefacts being created as part of Run/Walk, which leads us into a situation where it is difficult to keep the build artefacts away from the sentinel. We need to change this up and say, we create build artefacts, probably starting off with the director as part of Walk and Run, directly in Walk/Run, which subsequently results in the sentinel being created at a later stage.

The Configure method is where we handle the client specified options, for primary, we get the default options and apply the options specified on to. For resume, load from file and apply to the default options.

So in the above snippets, we don't create the sentinel until after Configure. Anything up to this point are the disposable builder artfacts suchs as the director and other factory objects. Doing it this way will ensure that the builders will not exist after construction.

@plastikfan
Copy link
Contributor Author

plastikfan commented May 14, 2024

As I continue down this route, I still feel myself making the wrong decisons. The next trap door is creating a director that implements an interface with 2 methods along the lines of this:

type Sync interface {
	Primary(root string, client core.Client) PrimaryDriver
	Resume(from string, strategy enums.ResumeStrategy) ResumeDriver
}

What I want is a singlular director that can be used for all scenarios, that uses child builders to create the end result. What I find is that I'm being led down a path where we have 2 directors, which is NOT what I want.

Perhaps we use functional composition to get a better result. Ie, we call a function, that returns a function (extent), then returns another function (sync). By the time we have called functions for the extent and the sync, we have an entity that can do the required construction; but even this approach is not desireable from the client's perspective as we would end up with an api that is not intiutive to use, eg

	traverse.Walk()(/*primary-args*/)(/*sync-args*/)

The above does not look good as we now don't have the method calls Primary/Resume, instead we invoke anonymnous functions.

So, I think I'm gonna have to accept, having 2 directors, one that can build a sequential sync and the other a reactive sync. Both of these builders require the extent builder (actually this dependency only exists for resume, not primary).

Wait a minute, at the highest level, we should start off with the fact that we need to invoke Walk/Run and end up with a Navigator.

I am being hampered by the fear of creating a set of absractions that theoretically would cause the exponential problem; ie the possibility in the future of requiring new abstractions being added. But in this case, this can't happen. Ie, we only ever need a navigator that can either be sequential/concurrent. There are no other possibilites. Similarly in the other dimension of extent, there is only the possibility to perform a full run (primary) or resume (contine from a previous terminated run). There are no other posibilites, so don't be hampered by this. Its ok to have say four navigators that cater to each of the comibantions, but actually what I really want, is to keep the extent (primary/resume) separate from the sync (sequential/reactive). Since our api starts off in the extent domain (ie we have top level factory functions Walk and Run), the factories we require must reflect this.

So we have a NavigatorFactory interface that can be implemented either by a linear(Walk) or reactor(Run) factory objects.

These factories should have a build method (which means build sync) that accpets the extent builder.

Or, the method should be Extent that takes an extent enum; so we replace Primary/Resume, with Extent and an enum discriminator.

Perhaps we rename Extent to something else as this may not convery to the client what is happening, but this is the name we use for now for lack of a better alternative.

@plastikfan
Copy link
Contributor Author

plastikfan commented May 15, 2024

I don't think I should be using rx at all. I've realised that the only thing I was using it for was to try and imprive the imlementation of the filtering functionality, but this was not a good idea. You just need to improve the filters application process and you don't need rx for that and trying to shoehorn that in is making me make more bad decisions. Just formalise the filter decoration process that easily supports hibernation.

Also, I can make the kernel package internel, which is a relief, because it frees me up to export items that are only meant for internal use.

plastikfan added a commit that referenced this issue May 15, 2024
plastikfan added a commit that referenced this issue May 15, 2024
plastikfan added a commit that referenced this issue May 15, 2024
@plastikfan plastikfan linked a pull request May 15, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
1 participant