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

[RSDK-9879] - Make datamanger sync dep implcit #36

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Additionally, make sure to add your configured data manager service to the `depe
| Attribute | Sub-Attribute | Type | Inclusion | Description |
|-----------------|-------------------|---------|-----------|---------------------------------------------------------------------------------------------------|
| `camera` | | string | optional | Name of the source camera to read images from. If not provided, video-store will not save video. |
| `sync` | | string | required | Name of the dependency datamanager service. |
| `sync` | | string | optional | Name of the dependency datamanager service. |
| `storage` | | object | required | |
| | `segment_seconds` | integer | optional | Length in seconds of the individual segment video files. |
| | `size_gb` | integer | required | Total amount of allocated storage in gigabytes. |
Expand Down
35 changes: 11 additions & 24 deletions cam/cam.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"go.viam.com/rdk/logging"
"go.viam.com/rdk/pointcloud"
"go.viam.com/rdk/resource"
"go.viam.com/rdk/services/datamanager"
rutils "go.viam.com/rdk/utils"
"go.viam.com/utils"
)
Expand Down Expand Up @@ -104,18 +105,17 @@ func (cfg *Config) Validate(path string) ([]string, error) {
if cfg.Storage.SizeGB == 0 {
return nil, utils.NewConfigValidationFieldRequiredError(path, "size_gb")
}
if cfg.Sync == "" {
return nil, utils.NewConfigValidationFieldRequiredError(path, "sync")
}
if cfg.Framerate < 0 {
return nil, fmt.Errorf("invalid framerate %d, must be greater than 0", cfg.Framerate)
}
// This allows for an implicit camera dependency so we do not need to explicitly
// add the camera dependency in the config.
dependencies := []string{}
if cfg.Sync != "" {
dependencies = append(dependencies, cfg.Sync)
}
if cfg.Camera != "" {
return []string{cfg.Camera}, nil
dependencies = append(dependencies, cfg.Camera)
}
return []string{}, nil
return dependencies, nil
}

func init() {
Expand Down Expand Up @@ -150,7 +150,7 @@ func newvideostore(
cameraAvailable := true
vs.cam, err = camera.FromDependencies(deps, newConf.Camera)
if err != nil {
vs.logger.Error("failed to get camera from dependencies, video-store will not be storing video", err)
vs.logger.Errorf("failed to get camera from dependencies, video-store will not be storing video: %v", err)
cameraAvailable = false
}

Expand Down Expand Up @@ -197,22 +197,9 @@ func newvideostore(

// Check for data_manager service dependency.
// TODO(seanp): Check custom_sync_paths if not using default upload_path in config.
syncFound := false
for key, dep := range deps {
if key.Name == newConf.Sync {
if dep.Name().API.Type.String() != "rdk:service" {
return nil, fmt.Errorf("sync service %s is not a service", newConf.Sync)
}
if dep.Name().API.SubtypeName != "data_manager" {
return nil, fmt.Errorf("sync service %s is not a data_manager service", newConf.Sync)
}
logger.Debugf("found sync service: %s", key.Name)
syncFound = true
break
}
}
if !syncFound {
return nil, fmt.Errorf("sync service %s not found", newConf.Sync)
_, err = datamanager.FromDependencies(deps, newConf.Sync)
if err != nil {
vs.logger.Errorf("failed to get sync service from dependencies, video-store will not upload saved clips to the cloud: %v", err)
}

// Create concater to handle concatenation of video clips when requested.
Expand Down
117 changes: 116 additions & 1 deletion tests/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,101 @@ func TestModuleConfiguration(t *testing.T) {
]
}`, fullModuleBinPath)

// Implicit sync dependency
config8 := fmt.Sprintf(`
{
"components": [
{
"name": "video-store-1",
"namespace": "rdk",
"type": "camera",
"model": "viam:video:storage",
"attributes": {
"camera": "fake-cam-1",
"sync": "data_manager-1",
"storage": {
"size_gb": 10,
"segment_seconds": 30,
"upload_path": "/tmp",
"storage_path": "/tmp"
},
"video": {
"preset": "ultrafast"
}
}
},
{
"name": "fake-cam-1",
"namespace": "rdk",
"type": "camera",
"model": "fake",
"attributes": {}
}
],
"services": [
{
"name": "data_manager-1",
"namespace": "rdk",
"type": "data_manager",
"attributes": {
"additional_sync_paths": [],
"capture_disabled": true,
"sync_interval_mins": 0.1,
"capture_dir": "",
"tags": []
}
}
],
"modules": [
{
"type": "local",
"name": "video-storage",
"executable_path": "%s",
"log_level": "debug"
}
]
}`, fullModuleBinPath)

// No sync dependency
config9 := fmt.Sprintf(`
{
"components": [
{
"name": "video-store-1",
"namespace": "rdk",
"type": "camera",
"model": "viam:video:storage",
"attributes": {
"camera": "fake-cam-1",
"storage": {
"size_gb": 10,
"segment_seconds": 30,
"upload_path": "/tmp",
"storage_path": "/tmp"
},
"video": {
"preset": "ultrafast"
}
}
},
{
"name": "fake-cam-1",
"namespace": "rdk",
"type": "camera",
"model": "fake",
"attributes": {}
}
],
"modules": [
{
"type": "local",
"name": "video-storage",
"executable_path": "%s",
"log_level": "debug"
}
]
}`, fullModuleBinPath)

t.Run("Valid Configuration Successful", func(t *testing.T) {
timeoutCtx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
Expand Down Expand Up @@ -535,7 +630,7 @@ func TestModuleConfiguration(t *testing.T) {
test.That(t, err, test.ShouldBeNil)
})

t.Run("Fails Configuration No DataManager", func(t *testing.T) {
t.Run("Specified Sync Without DataManager Service Fails", func(t *testing.T) {
timeoutCtx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
r, err := setupViamServer(timeoutCtx, config6)
Expand All @@ -556,4 +651,24 @@ func TestModuleConfiguration(t *testing.T) {
_, err = camera.FromRobot(r, videoStoreComponentName)
test.That(t, err, test.ShouldBeNil)
})

t.Run("Implicit Sync Dependency Succeeds", func(t *testing.T) {
timeoutCtx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
r, err := setupViamServer(timeoutCtx, config8)
test.That(t, err, test.ShouldBeNil)
defer r.Close(timeoutCtx)
_, err = camera.FromRobot(r, videoStoreComponentName)
test.That(t, err, test.ShouldBeNil)
})

t.Run("No Sync Dependency Succeeds", func(t *testing.T) {
timeoutCtx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
r, err := setupViamServer(timeoutCtx, config9)
test.That(t, err, test.ShouldBeNil)
defer r.Close(timeoutCtx)
_, err = camera.FromRobot(r, videoStoreComponentName)
test.That(t, err, test.ShouldBeNil)
})
}