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

cmd: make dsn a non-mandatory flag to allow use of Postgres environment variables #54

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

scottt
Copy link
Contributor

@scottt scottt commented Aug 10, 2023

Description

psql and programs using libpq allows users to use the environment variables PGHOST and friends to specify the connection information. This PR implements the same behavior for pg-schema-diff by making the DSN flag non-mandatory.

Motivation

  • We already set the Postgres environment variables in our development and deployment environments.
  • The Javascript and Golang libraries we use honor the variables.
  • This change makes pg-schema-diff "just work" in our workflow.

Implementation

  • pg-schema-diff's (connFlags).parseConnConfig() calls pgx.ParseConfig(), which calls pgconn.ParseConfigWithOptions()

  • pgconn.ParseConfigWithOptions already reads the Postgres environment variables.

Testing

  • I've tested the change manually on the command line and it works as expected

@CLAassistant
Copy link

CLAassistant commented Aug 10, 2023

CLA assistant check
All committers have signed the CLA.

@scottt scottt force-pushed the make-dsn-flag-non-mandatory branch from 860a08e to c866f0a Compare August 10, 2023 03:52
Copy link
Collaborator

@bplunkett-stripe bplunkett-stripe left a comment

Choose a reason for hiding this comment

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

This is super nice. I didn't realize pgx read from standard libpq style environment variables. Thanks for making this change!

Two small suggestions to round out the feature a bit:

Thanks again for contributing! 🎉

@scottt scottt force-pushed the make-dsn-flag-non-mandatory branch from c866f0a to f3c5658 Compare August 10, 2023 09:34
@scottt
Copy link
Contributor Author

scottt commented Aug 10, 2023

@bplunkett-stripe , I've incorporated the comments in f3c5658 which makes pg-schema-diff output:

2023/08/10 17:33:02 [WARNING] DSN flag not set. Using libpq environment variables and default values.

I've again manually tested pg-schema-diff from the command line and the new warning and database connection works as expected.

Copy link
Collaborator

@bplunkett-stripe bplunkett-stripe left a comment

Choose a reason for hiding this comment

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

LGTM! Solid set of changes

@bplunkett-stripe bplunkett-stripe merged commit c082d55 into stripe:main Aug 10, 2023
@scottt scottt deleted the make-dsn-flag-non-mandatory branch August 10, 2023 17:52
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