Skip to content
This repository has been archived by the owner on Mar 24, 2023. It is now read-only.

move some interfaces around #260

Merged
merged 1 commit into from
Aug 7, 2018
Merged

move some interfaces around #260

merged 1 commit into from
Aug 7, 2018

Conversation

dexhorthy
Copy link
Member

What I Did

Move some Daemon interface methods into a StatusReceiver interface, to be used for V2, replace Planner & Renderer Daemon
field with this narrower interface.

I played with a few other approaches to this, like taking the type doWithProgress func(chan interface{}, log.Logger) and
trying to pull that up the call stack as a parameter, but I think narrowing the daemon interface is going to be the quickest path
to victory.

I imagine once statusonly.StatusReceiver is done implementing daemontypes.StatusReceiver

  • instances will be created in a v2 route handler (maybe by a DI'd factory)
  • instance will have a channel for progress for that request,
  • instance will be set on a Planner or Renderer (made by a DI'd factory), and then
  • the v2 handler will read off the channel to set the status for a particular step ID, probably in a map[stepID]Progress
    or something.

How I Did it

  • pull out methods into status receiver:
type StatusReceiver interface {
    SetProgress(Progress)
    ClearProgress()
    PushStreamStep(context.Context, <-chan Message)
    SetStepName(context.Context, string)
}
  • Daemon implements StatusRecevier
type Daemon interface {
	StatusReceiver
}
  • Change Planner.Daemon to a StatusReceiver (dig still gives it a full Daemon for now)
  • Change Render.Daemon to a StatusReceiver (dig still gives it a full Daemon for now)
  • dig injects a func() Planner instead of a Planner, this will let us create an instance per request and set its StatusReceiver (Renderer will probably need this too, but its not done yet)
  • Also, randomly, remove a method from state.State interface that is superfluous now that we have state.State#Versioned()

How to verify it

All refactoring here still, this stuff has pretty decent coverage so far.

…V2 router, also replace state.ContentSHA() with state.Versioned().V1.ContentSHA

What I Did
------------

Move some Daemon interface methods into a `StatusReceiver` interface, to be used for V2, replace Planner & Renderer `Daemon`
field with this narrower interface.

I played with a few other approaches to this, like taking the type `doWithProgress func(chan interface{}, log.Logger)` and
trying to pull that up the call stack as a parameter, but I think narrowing the daemon interface is going to be the quickest path
to victory.

I imagine once `statusonly.StatusReceiver` is done implementing `daemontypes.StatusReceiver`
- instances will be created in a v2 route handler (maybe by a DI'd factory)
- instance will have a channel for progress for that request,
- instance will be set on a Planner or Renderer (made by a DI'd factory), and then
- the v2 handler will read off the channel to set the status for a particular step ID, probably in a map[stepID]Progress
  or something.

How I Did it
------------

- pull out methods into status receiver:

```go
type StatusReceiver interface {
    SetProgress(Progress)
    ClearProgress()
    PushStreamStep(context.Context, <-chan Message)
    SetStepName(context.Context, string)
}

```

- Daemon implements StatusRecevier

```go
type Daemon interface {
	StatusReceiver
}
```

- Change Planner.Daemon to a StatusReceiver (dig still gives it a full Daemon for now)
- Change Render.Daemon to a StatusReceiver (dig still gives it a full Daemon for now)
- dig injects a `func() Planner` instead of a `Planner`, this will let us create an instance per request and set its StatusReceiver (`Renderer` will probably need this too, but its not done yet)
- Also, randomly, remove a method from `state.State` interface that is superfluous now that we have `state.State#Versioned()`

How to verify it
------------

All refactoring here still, this stuff has pretty decent coverage so far.

![](https://media.giphy.com/media/l2Je3n9VXC8z3baTe/giphy.gif)
@dexhorthy dexhorthy requested a review from areed August 7, 2018 19:36
@dexhorthy dexhorthy merged commit 7955ef9 into replicatedhq:master Aug 7, 2018
@dexhorthy dexhorthy deleted the dex/complete-render-with-progress-4 branch August 10, 2018 16:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants