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

Feature: Use Postgres env vars in absence of explicit config #190

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

crwr45
Copy link

@crwr45 crwr45 commented Aug 5, 2020

This allows the tap to be added into existing Postgres environments
more easily, and reduces configuration duplication.

The pre-existing config still takes precedence, and the defaults
in the absence of config and env vars are what they were before.

Signed-off-by: crwr45 [email protected]

Copy link
Collaborator

@AlexanderMann AlexanderMann left a comment

Choose a reason for hiding this comment

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

@awm33 will have a better sense of whether this falls in line with the Singer standard for things. Fwiw, in my setup, I have a small wrapper which throws together this config from env vars, but they're not the PG defaults at all, but rather something more like DB__HOST, DB__PORT, ...

target_postgres/__init__.py Show resolved Hide resolved
This allows the tap to be added into existing Postgres environments
more easily, and reduces configuration duplication.

The pre-existing config still takes precedence, and the defaults
in the absence of config and env vars are what they were before.

Signed-off-by: crwr45 <[email protected]>
@AlexanderMann
Copy link
Collaborator

@awm33 any movement here, anything that should be done?

@laurentS
Copy link
Collaborator

I think this would also help with security and following the 12 factor principles, as it removes the need for writing a password to a file

@ericboucher
Copy link
Collaborator

@crwr45 could you have a quick look and make sure the tests are passing? Thanks!

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.

4 participants