cleaning up, pt. 3 (#376)

sdk/device.go

@@ -276,27 +276,6 @@ func (device *Device) IsWritable() bool {
 	return device.handler.Write != nil
 }
 
-// fixme: device ID generation
-
-//// ID generates the deterministic ID for the Device using its config values.
-//func (device *Device) ID() string {
-//	if device.id == "" {
-//		protocolComp := ctx.deviceIdentifier(device.Data)
-//		device.id = utils.NewUID(device.Plugin, device.Kind, protocolComp)
-//	}
-//	return device.id
-//}
-//
-//// GUID generates a globally unique ID string by creating a composite
-//// string from the rack, board, and device UID.
-//func (device *Device) GUID() string {
-//	return utils.MakeIDString( // fixme
-//		"", //device.Location.Rack,
-//		"", //device.Location.Board,
-//		device.ID(),
-//	)
-//}
-
 // encode translates the Device to the corresponding gRPC Device message.
 func (device *Device) encode() *synse.V3Device {
 	var tags []*synse.V3Tag
@@ -315,38 +294,3 @@ func (device *Device) encode() *synse.V3Device {
 		// todo:  capabilities, outputs
 	}
 }
-
-//// ValidateDeviceConfigData validates the `Data` field(s) of a Device Config to
-//// ensure that they are correct. The `Data` fields are plugin-specific, so its
-//// up to the user to provide us with a validation function.
-//func (config *DeviceConfig) ValidateDeviceConfigData(validator func(map[string]interface{}) error) *errors.MultiError {
-//	multiErr := errors.NewMultiError("device config 'data' field validation")
-//
-//	for _, device := range config.Devices {
-//		// Verify that the DeviceKind Instances' `Data` field is correct
-//		for _, instance := range device.Instances {
-//			err := validator(instance.Data)
-//			if err != nil {
-//				multiErr.Add(err)
-//			}
-//			// Instance Outputs can have their own data too. Verify instance
-//			// output data.
-//			for _, output := range instance.Outputs {
-//				err := validator(output.Data)
-//				if err != nil {
-//					multiErr.Add(err)
-//				}
-//			}
-//		}
-//
-//		// Device kind outputs can have their own data too. Verify the
-//		// device kind output data.
-//		for _, output := range device.Outputs {
-//			err := validator(output.Data)
-//			if err != nil {
-//				multiErr.Add(err)
-//			}
-//		}
-//	}
-//	return multiErr
-//}

sdk/device_manager.go

@@ -196,6 +196,8 @@ func (manager *deviceManager) AddDevice(device *Device) error {
 		device.handler = handler
 	}
 
+	// TODO: verify device Data with the custom verification fn
+
 	// If the device ID has not already been set, generate it and set
 	// it before adding it to the deviceManager.
 	if device.id == "" {

sdk/plugin.go

@@ -198,14 +198,6 @@ func (plugin *Plugin) Run() error {
 		os.Exit(0)
 	}
 
-	//// If the default health checks are enabled, register them now
-	//// fixme (etd) - this will move to the health manager
-	//if !plugin.config.Health.Checks.DisableDefaults {
-	//	log.Debug("[sdk] registering default health checks")
-	//	health.RegisterPeriodicCheck("read buffer health", 30*time.Second, readBufferHealthCheck)
-	//	health.RegisterPeriodicCheck("write buffer health", 30*time.Second, writeBufferHealthCheck)
-	//}
-
 	// Run the plugin.
 	return plugin.run()
 }

sdk/scheduler.go

@@ -553,6 +553,34 @@ func (scheduler *Scheduler) write(writeCtx *WriteContext) {
 	}()
 
 	// Wait for the write to complete, or timeout.
+	// FIXME (etd): this does technically give us a timeout, where we will stop
+	//  waiting for the write after the given timeout and just mark the transaction
+	//  dead, but thats kinda all this does.
+	//
+	//  from an upstream perspective, this is fine.. we want to be able to say
+	//  "we expect this write to complete after N time, afterwards, timeout and consider
+	//  it failed".
+	//
+	//  from the backend, it does present this to the frontend consumer, but it doesn't
+	//  actually stop the write from happening, so while we have "timed out", the write
+	//  is still going on in the background and could eventually resolve, which is
+	//  not at all what we want to do.
+	//
+	//  this could be resolved by passing in a context and cancelling, but uhh.. its
+	//  complicated and the internet has many mixed feeling-ed blog posts about it.
+	//
+	//  this relates to #365. I'll need to spend more time thinking this through.
+	//  essentially the issue lies in the fact that context cancellation relies on the
+	//  fact that you can terminate the work at some point (e.g. if iterating in an
+	//  infinite loop.., emitting from a channel, ...), then you can wait on the context
+	//  done signal, wait for the fn to finish up, and then you're done. to me, it seems
+	//  like the write will need to be waited on either way, so i'm not sure there is
+	//  a benefit to using a cancellation context unless writing is handled in a different
+	//  manner..
+	//
+	//  it seems like having a cancelation context could be useful if there is some
+	//  retry logic on the write, but thats mostly it..
+
 	var err error
 	select {
 	case writeErr := <-writer:

sdk/state_manager.go

@@ -243,23 +243,3 @@ func (manager *StateManager) GetReadings() map[string][]*output.Reading {
 	}
 	return readings
 }
-
-func (manager *StateManager) NewTransaction() *transaction {
-	return nil
-}
-
-func (manager *StateManager) UpdateTransactionStatusPending() {
-
-}
-
-func (manager *StateManager) UpdateTransactionStatusWriting() {
-
-}
-
-func (manager *StateManager) UpdateTransactionStatusDone() {
-
-}
-
-func (manager *StateManager) UpdateTransactionStatusError() {
-
-}