-
Notifications
You must be signed in to change notification settings - Fork 76
Start supporting barman #202
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
Conversation
internal/flypg/ssh.go
Outdated
return fmt.Errorf("failed to write cert: %s", err) | ||
} | ||
|
||
if err := writeSshConfig(); err != nil { |
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.
Nit: writeSSHConfig vs. writeSsh for consistency.
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.
Awesome work, just a few comments.
internal/flybarman/node.go
Outdated
"github.com/fly-apps/postgres-flex/internal/flypg/admin" | ||
) | ||
|
||
var dataDir = "/data" |
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.
You can just wrap this:
var (
dataDir = "/data"
barmanConfigFile = "..."
...
)
Any tests you can add for this would also be super helpful! |
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.
looks great!
@davissp14 added some tests for config and unified configs into the main node struct |
A release workflow appears to have been triggered, but no release has actually been created yet. Can you make a release for this? Creating a release helps me because I subscribe to it and regularly update the image by myself! |
Close #90
MUST
Maybe okay to do later
fly pg restart
unless a flag is passed and maybe make it restart for last