-
Notifications
You must be signed in to change notification settings - Fork 93
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
Update readme for setting PGPASSWORD when unable to include password in UTA_DB_URL #635
Conversation
Hi @reece , GitHub is not allowing me to add reviewers to this PR. Would you be able to take a look at this? TIA |
setup.py
Outdated
@@ -53,6 +53,7 @@ | |||
"parsley", | |||
"psycopg2", | |||
"six", | |||
"boto3" |
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.
maybe this should be an optional dependency?
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.
@bmrobin you're right! I just pushed this change in my latest commit. I don't really like adding the import inside the conditional, but didn't know where else to put the new module that contains this code (didn't know if the extras or utils directory would be the best place for it). Let me know if you have any other suggestions!
@korikuzma @bmrobin : I think it would be more conventional and cleaner to convey the password in an environment variable. Alternatively, Either way, my preference is make keep environment configuration as fully the caller's responsibility. Am I missing something? |
Hi @reece , thank you for the suggestions to help clean up this PR! We are unable to set the UTA_DB_URL with the generated db auth token since it is parsed incorrectly. I went with the route of using |
Hi @korikuzma . I slightly misled you: I should have written With that change, no code change is required at all. If you don't provide a password, libpq uses In other words, I think this entire PR can be whittled down to the the README comment to set
If you agree that this works, please remove the code change. Sorry for the underscore. :-/ |
Hi @reece , I tried your suggestion but am getting a |
That's surprising. Here's a session that I think demonstrates that this works.
If PGPASSWORD isn't working, it's either misspelled or it's being stripped from your environment (e.g., by docker or sudo, both of which provide mechanisms for conveying selected environment variables). Does that help? |
Hi @reece , I double checked that Are we unable to keep this line? (Changing
|
Hi @korikuzma - I'd like to get to the bottom of this. The postgresql authentication mechanism is extremely well-designed. We should be using it, and adding a different mechanism is likely to create confusion. Please try this:
Then try against your own RDS instance. I am certain that this mechanism should work; if it doesn't, there's an underlying issue that should be fixed directly rather than by workaround in hgvs. |
Hi @reece , So I am able to connect to our RDS instance using these commands:
|
Thanks @korikuzma. Okay, please try this in an environment with ipython and hgvs installed, and no ~./.pgpass
In my opinion, this kinda proves that PGPASSWORD should be working. If it doesn't, there's something happening in your environment outside of hgvs. |
@reece oh my gosh. I'm so sorry. I was looking at your UTA_DB_URL and realized that there is no colon after the user (I was adding this). 🤦♀️ I'll remove my changes and just update the readme. |
No problem. I like puzzles so this was actually kinda fun for me! |
We would like to be able to connect to our UTA DB that is hosted on AWS via a generated auth token. We ran into issues trying to do this when trying to use vrs-python's translate_to method in variation normalizer as seen in this issue.