search

rightfold / purescript-postgresql-client

https://pursuit.purescript.org/packages/purescript-postgresql-client
BSD 3-Clause "New" or "Revised" License
35 stars 20 forks source link

Parse connection postgres uri #46

Closed isaactovsky closed 4 years ago

isaactovsky commented 4 years ago

Hi, first, thanks for this library. Some deployment platforms offer to include postgres service (heroku one example) and then expose the postgres connection uri via environment variable. This is a util function Uri -> Maybe Pool that i found personally useful in my project.

paluh commented 4 years ago

Sorry for the late response - I'm not sure why I have not received notification about your PR.

This is great addition. Thanks for implementing this parser and tests! I've found some minor formatting issues - sorry for being so pedantic. If you don't want to fix them I can merge and fix them myself :-)

P.S. If you are using PostgreSQL with PureScript you can be also interested in purescript-selda - please check it out!

isaactovsky commented 4 years ago

Hi, on the contrary, I appreciate your feedback and guidelines about formatting records. I'm getting used to writing purescript :)

isaactovsky commented 4 years ago

Hi, @paluh Any updates about when this will be merged?

paluh commented 4 years ago

Holy cow! I'm really sorry @isaactovsky for this delay. I had not clicked "submit review" button and it was hanging so long. I added another comment related to formatting. I hope you don't mind. Of course if you don't have time to fix this minor issue I'm going to merge it and provide a fix myself.

paluh commented 4 years ago

Thanks a lot!

paluh commented 4 years ago

It seems that github doesn't like block splitting - this breaks release section. I'm going to test this myself before the next release.

paluh commented 4 years ago

@isaactovsky I've checked now node-postgresql documentation and it seems that it supports postgresql uri on its own level. I wonder if we want to parse it on our side an maintain this parsing (for example now we have an issue with sslmode parsing).

Maybe we should drop parser and go with a sum type like:

data PoolConfiguration = PgConfigurationlUri String | PgConfig { ... }

Would this kind of solution work for you?

isaactovsky commented 4 years ago

@paluh, yeah sure, go for it. I should have checked node-postgresql for the uri stuff myself in the first place :)