Feature:Postgres commands through http #893
Conversation
cmd/postgres.go
Outdated
| if !isPostgresApp(&app.ImageDetails) { | ||
| return fmt.Errorf("%s is not a postgres app", cmdCtx.AppName) | ||
| } | ||
| // if !isPostgresApp(&app.ImageDetails) { |
There was a problem hiding this comment.
Are we checking this somewhere else?
There was a problem hiding this comment.
@davissp14 I only removed this temporary for testing, cause pg apps deployed with fly deploy fail to pass this checks
cmd/postgres.go
Outdated
| } | ||
|
|
||
| pgCmd := NewPostgresCmd(cmdCtx, app, dialer) | ||
| // pgCmd := NewPostgresCmd(cmdCtx, app, dialer) |
There was a problem hiding this comment.
We can probably just remove the commented code.
There was a problem hiding this comment.
Yeah will do, Just wanted to get a quick reaction before going to far with it.
pkg/flypg/http.go
Outdated
| } | ||
| defer res.Body.Close() | ||
|
|
||
| if res.Status > "299" { |
There was a problem hiding this comment.
What are the expected status codes? Also, looks like you're comparing an int with a string here.
There was a problem hiding this comment.
True I'll work on it.
There was a problem hiding this comment.
Will we be working with any valid status codes, other than 200 and 201?
pkg/flypg/pg.go
Outdated
| return false, nil | ||
| } | ||
|
|
||
| func (c *Client) GrantAccess(ctx context.Context, dbName, userName string) error { |
There was a problem hiding this comment.
This should probably be coupled with CreateUser. Given the complexity surrounding user permissions, we shouldn't expose this as its own endpoint.
pkg/flypg/pg.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func (c *Client) RevokeAccess(ctx context.Context, dbName, userName string) error { |
There was a problem hiding this comment.
This should probably be coupled with DeleteUser. Like GrantAccess, let's not expose this as its own endpoint.
v0.0.19ofpostgres-haThis PR goes hand in hand with fly-apps/postgres-ha#64