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.

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

Allow fly pg connect to work with custom images #3680

Merged
merged 7 commits into from
Jul 8, 2024
Merged

Conversation

benwaffle
Copy link
Contributor

@benwaffle benwaffle commented Jun 27, 2024

This is relevant in development - e.g. if you run fly deploy -a <my-db> in postgres-flex

@benwaffle benwaffle requested a review from davissp14 June 27, 2024 05:52
@davissp14
Copy link
Contributor

davissp14 commented Jun 27, 2024

You can currently specify FLY_DEV=1 to bypass the custom image restriction.

@benwaffle
Copy link
Contributor Author

Oh, didn't notice that. This PR makes it unnecessary. Any thoughts about getting rid of FLY_DEV? I don't see any risk here.

@davissp14
Copy link
Contributor

davissp14 commented Jun 28, 2024

The pg connect command calls https://github.com/fly-apps/postgres-flex/blob/master/bin/connect

The main problem is that we can't assume custom images have this binary.

@benwaffle
Copy link
Contributor Author

Today if you run fly pg connect on a custom image, you get this error: Error: Malformed version: ustom.

Anyone who's hacking on postgres-flex is likely to have the connect binary. In the rare case that they have removed it, with this PR they will see:

❯ f pg connect -a dbdbdb
Connecting to fdaa:1:a290:a7b:2f1:f87a:2dab:2... complete
exec: "connect": executable file not found in $PATH
Error: ssh shell: wait: remote command exited without exit status or exit signal

@davissp14
Copy link
Contributor

davissp14 commented Jun 29, 2024

I'm don't think we should remove the FLY_DEV check, but i'm cool opening up the fly pg connect command.

@davissp14 davissp14 self-requested a review June 29, 2024 22:40
@benwaffle
Copy link
Contributor Author

For the record, this changes hasRequiredVersionOnMachines, which is used in nearly all fly pg commands.

@benwaffle benwaffle merged commit 92b9ea9 into master Jul 8, 2024
34 checks passed
@benwaffle benwaffle deleted the pg-connect-custom branch July 8, 2024 20:19
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.

None yet

2 participants