Skip to content

feat(runners): add workload agent_state#47

Open
emerson-gray wants to merge 2 commits intomainfrom
feat/workload-agent-state
Open

feat(runners): add workload agent_state#47
emerson-gray wants to merge 2 commits intomainfrom
feat/workload-agent-state

Conversation

@emerson-gray
Copy link
Copy Markdown
Contributor

Implements Workload.agent_state (processing/idle) to separate container lifecycle from agent activity.

Changes:

  • DB migration: add workloads.agent_state (default processing) + indexes.
  • Populate Workload.agent_state in proto conversion.
  • TouchWorkload: atomically flips idle -> processing and only emits workload.updated on that transition.
  • Agent Activity Sweep: flips processing -> idle when status=running and last_activity_at is older than KEEPALIVE_GRACE.
  • Notifications: workload.updated payload now includes agent_state.

Config:

  • AGENT_ACTIVITY_SWEEP_INTERVAL (default 5s)
  • KEEPALIVE_GRACE (default 25s)

Notes:

  • Depends on agynio/api PR #139 (proto field + enum).
  • Unit tests updated (CGO_ENABLED=0 go test ./...).

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes:

  • [major] is unqualified but used in JOIN queries (sort-by-runner), which will be ambiguous in Postgres.
  • [major] Please add unit coverage for the idle→processing transition + emitted notifications/payload.

Minor note inline on config/sweep invariants.

Comment thread internal/server/workloads.go Outdated
Comment thread internal/server/agent_activity_sweep.go
Comment thread internal/server/workloads_test.go
@noa-lucent
Copy link
Copy Markdown

FYI: my review summary in the REQUEST_CHANGES body was mangled by shell backticks.

Blockers:

  • workloadColumns is unqualified but used in JOIN queries (sort-by-runner), which will be ambiguous in Postgres.
  • Please add unit coverage for the TouchWorkload idle→processing transition + emitted notifications/payload.

@casey-brooks
Copy link
Copy Markdown
Contributor

Summary

  • Switch list workloads/volumes to filter fields and update pagination query builders; removed unused helpers to satisfy lint.
  • Adjusted workload log streaming request handling and sweep/config behavior; improved gRPC close error handling.
  • Added workload idle->processing notification coverage and updated list filter tests.

Testing

  • go test ./... (passed: all, failed: 0, skipped: 0)
  • golangci-lint run ./... (no issues)

Notes

  • Depends on api#139 for regenerated proto/agent_state fields; CI may need rerun after it merges.

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review: fixes look good. SELECT columns are now workload-qualified (avoids JOIN ambiguity) and there is new unit coverage for the idle→processing TouchWorkload notification path (incl agent_state in payload). Config now allows 0 to disable sweep and sweep treats context cancellation as normal shutdown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants