-
Notifications
You must be signed in to change notification settings - Fork 126
APP-10064: Add "continuous" scheduling option #5495
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
Changes from all commits
35beba9
b13884f
98f67fe
4fa3333
eaad9a5
e25e16a
ca0501d
503e408
93f3154
0f355cd
834ee04
668f3dd
ecba4ff
df48add
cdae0a8
99fb6b5
38d29f4
baba43f
073552e
84ad603
07e47a0
44edcc1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe there are tests associated with this file; worth adding a test there for roundtripping. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1032,6 +1032,9 @@ func JobsConfigToProto(jc *JobConfig) (*pb.JobConfig, error) { | |
| } | ||
| protoConfig.Command = command | ||
| } | ||
| if jc.LogConfiguration != nil { | ||
| protoConfig.LogConfiguration = &pb.LogConfiguration{Level: strings.ToLower(jc.LogConfiguration.Level.String())} | ||
| } | ||
|
Comment on lines
+1036
to
+1037
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. App will need an RDK bump (containing API bump) after this PR is merged for the web editor to support |
||
| return protoConfig, nil | ||
| } | ||
|
|
||
|
|
@@ -1049,5 +1052,14 @@ func JobsConfigFromProto(proto *pb.JobConfig, _ logging.Logger) (*JobConfig, err | |
| if proto.Command != nil { | ||
| jobConfig.Command = proto.Command.AsMap() | ||
| } | ||
| if proto.LogConfiguration != nil { | ||
| if proto.GetLogConfiguration() != nil { | ||
| level, err := logging.LevelFromString(proto.GetLogConfiguration().Level) | ||
| if err != nil { | ||
| level = logging.INFO | ||
| } | ||
| jobConfig.LogConfiguration = &resource.LogConfig{Level: level} | ||
| } | ||
| } | ||
| return jobConfig, nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import ( | |
| "time" | ||
|
|
||
| "github.com/pion/mediadevices/pkg/prop" | ||
| "go.uber.org/zap/zapcore" | ||
| "go.viam.com/test" | ||
| "go.viam.com/utils" | ||
| "go.viam.com/utils/testutils" | ||
|
|
@@ -71,6 +72,91 @@ func TestJobManagerDurationAndCronFromJson(t *testing.T) { | |
| }) | ||
| } | ||
|
|
||
| func TestLogLevelChange(t *testing.T) { | ||
| // This is created at debug level | ||
| logger, logs := logging.NewObservedTestLogger(t) | ||
|
|
||
| fakeSensorComponent := []resource.Config{ | ||
| { | ||
| Model: resource.DefaultModelFamily.WithModel("fake"), | ||
| Name: "sensor", | ||
| API: sensor.API, | ||
| }, | ||
| } | ||
|
|
||
| cfg := &config.Config{ | ||
| Components: fakeSensorComponent, | ||
| Jobs: []config.JobConfig{ | ||
| { | ||
| config.JobConfigData{ | ||
| Name: "fake sensor", | ||
| Schedule: "1s", | ||
| Resource: "sensor", | ||
| Method: "GetReadings", | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| cfgWarn := &config.Config{ | ||
| Components: fakeSensorComponent, | ||
| Jobs: []config.JobConfig{ | ||
| { | ||
| config.JobConfigData{ | ||
| Name: "fake sensor", | ||
| Schedule: "1s", | ||
| Resource: "sensor", | ||
| Method: "GetReadings", | ||
| LogConfiguration: &resource.LogConfig{ | ||
| Level: logging.WARN, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| cfgDebug := &config.Config{ | ||
| Components: fakeSensorComponent, | ||
| Jobs: []config.JobConfig{ | ||
| { | ||
| config.JobConfigData{ | ||
| Name: "fake sensor", | ||
| Schedule: "1s", | ||
| Resource: "sensor", | ||
| Method: "GetReadings", | ||
| LogConfiguration: &resource.LogConfig{ | ||
| Level: logging.DEBUG, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| ctx, ctxCancelFunc := context.WithCancel(context.Background()) | ||
| defer ctxCancelFunc() | ||
| lr := setupLocalRobot(t, ctx, cfg, logger) | ||
|
|
||
| time.Sleep(7 * time.Second) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any way to remove sleeps here? |
||
| test.That(t, logs.FilterMessage("Job triggered").FilterLevelExact(zapcore.DebugLevel).Len(), | ||
| test.ShouldBeGreaterThan, 2) | ||
| test.That(t, logs.FilterMessage("Job succeeded").FilterLevelExact(zapcore.DebugLevel).Len(), | ||
| test.ShouldBeGreaterThan, 2) | ||
|
|
||
| lr.Reconfigure(ctx, cfgWarn) | ||
| logs.TakeAll() | ||
| time.Sleep(7 * time.Second) | ||
| // update will let the previous job iteration complete first, so may be 1 or 0. | ||
| test.That(t, logs.FilterMessage("Job triggered").FilterLevelExact(zapcore.DebugLevel).Len(), | ||
| test.ShouldBeLessThanOrEqualTo, 1) | ||
| test.That(t, logs.FilterMessage("Job succeeded").FilterLevelExact(zapcore.DebugLevel).Len(), | ||
| test.ShouldBeLessThanOrEqualTo, 1) | ||
|
|
||
| lr.Reconfigure(ctx, cfgDebug) | ||
| logs.TakeAll() | ||
| time.Sleep(7 * time.Second) | ||
| test.That(t, logs.FilterMessage("Job triggered").FilterLevelExact(zapcore.DebugLevel).Len(), | ||
| test.ShouldBeGreaterThan, 2) | ||
| test.That(t, logs.FilterMessage("Job succeeded").FilterLevelExact(zapcore.DebugLevel).Len(), | ||
| test.ShouldBeGreaterThan, 2) | ||
| } | ||
|
|
||
| func TestJobManagerHistory(t *testing.T) { | ||
| logger := logging.NewTestLogger(t) | ||
|
|
||
|
|
@@ -228,6 +314,119 @@ func TestJobManagerHistory(t *testing.T) { | |
| }) | ||
| } | ||
|
|
||
| // Test continuous mode, include switching to and from. | ||
| func TestJobContinuousSchedule(t *testing.T) { | ||
| logger := logging.NewTestLogger(t) | ||
|
|
||
| fakeSensorComponent := []resource.Config{ | ||
| { | ||
| Model: resource.DefaultModelFamily.WithModel("fake"), | ||
| Name: "sensor", | ||
| API: sensor.API, | ||
| }, | ||
| } | ||
|
|
||
| cfg := &config.Config{ | ||
| Components: fakeSensorComponent, | ||
| Jobs: []config.JobConfig{ | ||
| { | ||
| config.JobConfigData{ | ||
| Name: "fake sensor", | ||
| Schedule: "1s", | ||
| Resource: "sensor", | ||
| Method: "GetReadings", | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| cfgCron := &config.Config{ | ||
| Components: fakeSensorComponent, | ||
| Jobs: []config.JobConfig{ | ||
| { | ||
| config.JobConfigData{ | ||
| Name: "fake sensor", | ||
| Schedule: "*/1 * * * * *", | ||
| Resource: "sensor", | ||
| Method: "GetReadings", | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| cfgContinuous := &config.Config{ | ||
| Components: fakeSensorComponent, | ||
| Jobs: []config.JobConfig{ | ||
| { | ||
| config.JobConfigData{ | ||
| Name: "fake sensor", | ||
| Schedule: "continuous", | ||
| Resource: "sensor", | ||
| Method: "GetReadings", | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| ctx, ctxCancelFunc := context.WithCancel(context.Background()) | ||
| defer ctxCancelFunc() | ||
| lr := setupLocalRobot(t, ctx, cfg, logger) | ||
| o, _, addr := robottestutils.CreateBaseOptionsAndListener(t) | ||
| err := lr.StartWeb(ctx, o) | ||
| test.That(t, err, test.ShouldBeNil) | ||
| robotClient, err := rclient.New(ctx, addr, logger) | ||
| test.That(t, err, test.ShouldBeNil) | ||
| defer robotClient.Close(ctx) | ||
|
|
||
| // Start running in 1s duration mode. Expect last 2 latest success timestamps differ by > 900ms | ||
| testutils.WaitForAssertionWithSleep(t, time.Second, 10, func(tb testing.TB) { | ||
| tb.Helper() | ||
| ms, err := robotClient.MachineStatus(ctx) | ||
| test.That(tb, err, test.ShouldBeNil) | ||
| test.That(tb, len(ms.JobStatuses), test.ShouldEqual, 1) | ||
| jh, ok := ms.JobStatuses["fake sensor"] | ||
| test.That(tb, ok, test.ShouldBeTrue) | ||
| successes := jh.RecentSuccessfulRuns | ||
| test.That(tb, len(successes), test.ShouldBeGreaterThanOrEqualTo, 2) | ||
| if len(successes) >= 2 { | ||
| test.That(tb, successes[len(successes)-1].Sub(successes[len(successes)-2]), test.ShouldBeGreaterThan, 900*time.Millisecond) | ||
| } | ||
| }) | ||
|
|
||
| // Switch from duration to continuous. Should run more than 10x. Expect latest success timestamp - earliest < 900ms | ||
| lr.Reconfigure(ctx, cfgContinuous) | ||
| testutils.WaitForAssertionWithSleep(t, time.Second, 10, func(tb testing.TB) { | ||
| tb.Helper() | ||
| ms, err := robotClient.MachineStatus(ctx) | ||
| test.That(tb, err, test.ShouldBeNil) | ||
| test.That(tb, len(ms.JobStatuses), test.ShouldEqual, 1) | ||
| jh, ok := ms.JobStatuses["fake sensor"] | ||
| test.That(tb, ok, test.ShouldBeTrue) | ||
| successes := jh.RecentSuccessfulRuns | ||
| test.That(tb, ok, test.ShouldBeTrue) | ||
| // increase this if bumping history size | ||
| test.That(tb, len(successes), test.ShouldBeGreaterThanOrEqualTo, 10) | ||
| if len(successes) >= 2 { | ||
| test.That(tb, successes[len(successes)-1].Sub(successes[0]), test.ShouldBeLessThan, 900*time.Millisecond) | ||
| } | ||
| }) | ||
|
|
||
| // Switch from continuous to 1s cron. Expect last 2 latest success timestamps differ by > 900ms | ||
| lr.Reconfigure(ctx, cfgCron) | ||
| testutils.WaitForAssertionWithSleep(t, time.Second, 10, func(tb testing.TB) { | ||
| tb.Helper() | ||
| ms, err := robotClient.MachineStatus(ctx) | ||
| test.That(tb, err, test.ShouldBeNil) | ||
| test.That(tb, len(ms.JobStatuses), test.ShouldEqual, 1) | ||
| jh, ok := ms.JobStatuses["fake sensor"] | ||
| test.That(tb, ok, test.ShouldBeTrue) | ||
| successes := jh.RecentSuccessfulRuns | ||
| test.That(tb, ok, test.ShouldBeTrue) | ||
| // History still contains runs from prev. If stored size is 10, we still expect 10 here. | ||
| test.That(tb, len(successes), test.ShouldBeGreaterThanOrEqualTo, 10) | ||
| if len(successes) >= 2 { | ||
| test.That(tb, successes[len(successes)-1].Sub(successes[len(successes)-2]), test.ShouldBeGreaterThan, 900*time.Millisecond) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| func TestJobManagerConfigChanges(t *testing.T) { | ||
| logger := logging.NewTestLogger(t) | ||
| model := resource.DefaultModelFamily.WithModel(utils.RandomAlphaString(8)) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since
LogConfig's are now shared between jobs and resources, mayberesourceis no longer the correct package.