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.

Sign up for GitHub

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

Jump to bottom

GetStatus() should always return just the active controllers/runners/ingress paths #1282

Merged
merged 3 commits into from
Apr 17, 2024
Merged

GetStatus() should always return just the active controllers/runners/ingress paths #1282

merged 3 commits into from
Apr 17, 2024

Conversation

deniseli
Copy link
Contributor

@deniseli deniseli commented Apr 17, 2024 •
edited
Loading

Fixes issue #1171

Generated file changes produced by running ./bin/sqlc generate

Manually tested by running ftl status and ftl serve, the two commands that use GetStatus() via StatusRequest:

  1. In tab 1, ran ftl dev ./examples/go. Confirmed everything started as expected.
  2. Opened a new tab 2, ran ftl status. Confirmed everything got returned.
  3. Went back to tab 1, Ctrl+C.
  4. Went back to tab 2 and ran ftl status again. Confirmed got ftl: error: unavailable: dial tcp 127.0.0.1:8892: connect: connection refused.
  5. Went back to tab 1 and ran ftl serve. Confirmed serving executed as expected.
  6. Went back to tab 2 and ran ftl status again. Confirmed everything got returned again.

@deniseli deniseli requested a review from a team as a code owner April 17, 2024 00:08
@deniseli deniseli requested review from worstell and removed request for a team April 17, 2024 00:08
@github-actions github-actions bot changed the title Dli/active only getstatus feat(Dli/active): only getstatus Apr 17, 2024
@deniseli deniseli changed the title feat(Dli/active): only getstatus GetStatus() should always return just the active controllers/runners/ingress paths Apr 17, 2024
@alecthomas alecthomas mentioned this pull request Apr 16, 2024
Open
alecthomas
alecthomas approved these changes Apr 17, 2024
View reviewed changes
Copy link
Collaborator

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

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

LGTM thanks @deniseli

backend/controller/sql/queries.sql
@@ -427,11 +426,10 @@ WITH matches AS (
SELECT COUNT(*)
FROM matches;

-- name: GetControllers :many
-- name: GetActiveControllers :many
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@deniseli deniseli merged commit cff05fe into main Apr 17, 2024
11 checks passed
@deniseli deniseli deleted the dli/active-only-getstatus branch April 17, 2024 00:16
@deniseli deniseli mentioned this pull request Apr 17, 2024
Merged
deniseli added a commit that referenced this pull request Apr 17, 2024
@deniseli
fix: ControllerState proto enum references, now that the enum has bee…
34650d7 
…n deleted from the proto file (#1288)

Fixes what PR #1282 broke.
Removed stale references to the `ControllerState` proto enum, then ran
`buf generate`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alecthomas alecthomas alecthomas approved these changes

@worstell worstell Awaiting requested review from worstell worstell was automatically assigned from TBD54566975/ftl-team

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@deniseli @alecthomas