Skip to content

Commit dc036c9

Browse files
committed
update notes
1 parent 038174e commit dc036c9

File tree

4 files changed

+6
-10
lines changed

4 files changed

+6
-10
lines changed

dbos/admin_server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func (as *adminServer) Start() error {
120120
func (as *adminServer) Shutdown(ctx context.Context) error {
121121
as.logger.Info("Shutting down admin server")
122122

123-
// XXX consider moving the grace period to DBOSContext.Shutdown()
123+
// Note: consider moving the grace period to DBOSContext.Shutdown()
124124
ctx, cancel := context.WithTimeout(ctx, _ADMIN_SERVER_SHUTDOWN_TIMEOUT)
125125
defer cancel()
126126

dbos/system_database.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,7 +1514,6 @@ func (s *sysDB) recv(ctx context.Context, input WorkflowRecvInput) (any, error)
15141514
functionName := "DBOS.recv"
15151515

15161516
// Get workflow state from context
1517-
// XXX these checks might be better suited for outside of the system db code. We'll see when we implement the client.
15181517
wfState, ok := ctx.Value(workflowStateKey).(*workflowState)
15191518
if !ok || wfState == nil {
15201519
return nil, newStepExecutionError("", functionName, "workflow state not found in context: are you running this step within a workflow?")
@@ -1534,7 +1533,6 @@ func (s *sysDB) recv(ctx context.Context, input WorkflowRecvInput) (any, error)
15341533
}
15351534

15361535
// Check if operation was already executed
1537-
// XXX this might not need to be in the transaction
15381536
checkInput := checkOperationExecutionDBInput{
15391537
workflowID: destinationID,
15401538
stepID: stepID,
@@ -1561,7 +1559,6 @@ func (s *sysDB) recv(ctx context.Context, input WorkflowRecvInput) (any, error)
15611559
}
15621560
defer func() {
15631561
// Clean up the condition variable after we're done and broadcast to wake up any waiting goroutines
1564-
// XXX We should handle panics in this function and make sure we call this. Not a problem for now as panic will crash the importing package.
15651562
cond.Broadcast()
15661563
s.notificationsMap.Delete(payload)
15671564
}()
@@ -1576,7 +1573,6 @@ func (s *sysDB) recv(ctx context.Context, input WorkflowRecvInput) (any, error)
15761573
}
15771574
if !exists {
15781575
// Wait for notifications using condition variable with timeout pattern
1579-
// XXX should we prevent zero or negative timeouts?
15801576
s.logger.Debug("Waiting for notification on condition variable", "payload", payload)
15811577

15821578
done := make(chan struct{})
@@ -1796,7 +1792,7 @@ func (s *sysDB) getEvent(ctx context.Context, input WorkflowGetEventInput) (any,
17961792
return nil, fmt.Errorf("failed to query workflow event: %w", err)
17971793
}
17981794

1799-
if err == pgx.ErrNoRows || valueString == nil { // XXX valueString should never be `nil`
1795+
if err == pgx.ErrNoRows || valueString == nil { // valueString should never be `nil`
18001796
// Wait for notification with timeout using condition variable
18011797
done := make(chan struct{})
18021798
go func() {

dbos/workflow.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func (h *workflowPollingHandle[R]) GetResult() (R, error) {
201201
}
202202
recordResultErr := h.dbosContext.(*dbosContext).systemDB.recordChildGetResult(h.dbosContext, recordGetResultInput)
203203
if recordResultErr != nil {
204-
// XXX do we want to fail this?
204+
// Note: we might want GetResult to return this error if it happens, instead of returning the real error to the program.
205205
h.dbosContext.(*dbosContext).logger.Error("failed to record get result", "error", recordResultErr)
206206
}
207207
}
@@ -282,7 +282,7 @@ func registerScheduledWorkflow(ctx DBOSContext, workflowName string, fn Workflow
282282
// Use Next if Prev is not set, which will only happen for the first run
283283
scheduledTime = entry.Next
284284
}
285-
wfID := fmt.Sprintf("sched-%s-%s", workflowName, scheduledTime) // XXX we can rethink the format
285+
wfID := fmt.Sprintf("sched-%s-%s", workflowName, scheduledTime)
286286
opts := []WorkflowOption{
287287
WithWorkflowID(wfID),
288288
WithQueue(_DBOS_INTERNAL_QUEUE_NAME),

dbos/workflows_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1414,7 +1414,7 @@ func TestSendRecv(t *testing.T) {
14141414
if result != "message1-message2-message3" {
14151415
t.Fatalf("expected received message to be 'message1-message2-message3', got '%s'", result)
14161416
}
1417-
// XXX This is not a great condition: when all the tests run there's quite some randomness to this
1417+
// FIXME This is not a great condition: when all the tests run there's quite some randomness to this
14181418
if time.Since(start) > 10*time.Second {
14191419
t.Fatalf("receive workflow took too long to complete, expected < 5s, got %v", time.Since(start))
14201420
}
@@ -2895,7 +2895,7 @@ func TestWorkflowTimeout(t *testing.T) {
28952895
}
28962896

28972897
// Check the deadline on the status was is within an expected range (start time + timeout * .1)
2898-
// XXX this might be flaky and frankly not super useful
2898+
// FIXME this might be flaky and frankly not super useful
28992899
expectedDeadline := start.Add(timeout * 10 / 100)
29002900
if status.Deadline.Before(expectedDeadline) || status.Deadline.After(start.Add(timeout)) {
29012901
t.Fatalf("expected workflow deadline to be within %v and %v, got %v", expectedDeadline, start.Add(timeout), status.Deadline)

0 commit comments

Comments
 (0)