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

fix: update pg-connection-string #2105

Closed
wants to merge 3 commits into from
Closed

Conversation

revolunet
Copy link
Contributor

Hi,

Use a recent pg-connection-string release that handle much more cases (ex: ssl)

Thanks

@brianc
Copy link
Owner

brianc commented Feb 19, 2020

hey thanks! I'll look at this tomorrow. Was out of town for a couple weeks.

@brianc
Copy link
Owner

brianc commented Feb 20, 2020

Hmm....do you know what breaking changes there were between the old version and this one? I definitely want to upgrade this, but I need to understand if this is going to be a breaking change for consumers of this library. That's fine if it is - I'll just need to bundle it up w/ the 8.0 release going out this weekend or so.

@revolunet
Copy link
Contributor Author

Hi,

quite hard to tell as author bumped from 0.1.3 to 2.0.0. Maybe @phated @hjr3 could give a hint ?

@hjr3
Copy link
Contributor

hjr3 commented Feb 20, 2020

We discussed this a bit here: iceddev/pg-connection-string#22

tl;dr there were no intentional breaking changes

@brianc
Copy link
Owner

brianc commented Feb 20, 2020

awesome! Thanks for the update. I'll audit the code just for double-checking purposes, run the tests, etc.

Also - if you're interested we can pull that module into this monorepo and keep it more lock-step versioned with the rest of node-postgres's stuff since it's one of the few direct dependencies pg takes outside of this monorepo. I'm happy to do maintenance on the module as well. Particularly around some ssl connection string improvements I'd like to make in the future.

let me know how you feel about that - I'm happy to do the work on that!

@brianc
Copy link
Owner

brianc commented Feb 20, 2020

Also...since we're not entirely sure there aren't any breaking changes I'd be best to target this pr at the bmc/8.0 branch. If you're cool with that I can make the change, or you can - just change the dropdown in the github ui!

@brianc
Copy link
Owner

brianc commented Feb 20, 2020

note: 8.0 should be going out this week or next, depending on when I get time to push the final ssl connection param changes out w/ tests - testing various ssl connections has been tricky to date, but I did some work recently on making it a bit easier including getting a self-signed cert locally & ensuring ssl connections are tested in travis.

@hjr3
Copy link
Contributor

hjr3 commented Feb 20, 2020

awesome! Thanks for the update. I'll audit the code just for double-checking purposes, run the tests, etc.

Also - if you're interested we can pull that module into this monorepo and keep it more lock-step versioned with the rest of node-postgres's stuff since it's one of the few direct dependencies pg takes outside of this monorepo. I'm happy to do maintenance on the module as well. Particularly around some ssl connection string improvements I'd like to make in the future.

let me know how you feel about that - I'm happy to do the work on that!

@brianc I do it is wise to move that dependency into this repo. I am not the owner of that repo, but I am the latest maintainer. The other maintainers are mostly inactive at this point. I too am happy to help however I can.

@brianc
Copy link
Owner

brianc commented Feb 20, 2020

Okay great! I'll get it moved over tomorrow provided I have time in the morning...otherwise...this weekend! Then I'll get it in the flow for the 8.0 branch release.

@DiegoRBaquero
Copy link

Any update on this? Me and other folks in @pagerinc would like to contribute in the ssl settings parsing.

@revolunet
Copy link
Contributor Author

Could you try this PR and confirm if it works as expected or not ?

@brianc
Copy link
Owner

brianc commented Apr 28, 2020

Any update on this? Me and other folks in @pagerinc would like to contribute in the ssl settings parsing.

ha yeah sorry i have soooo many tasks I have to juggle w/ this library...I drop stuff on the floor sometimes. I'll migrate it over here this morning.

@monteslu
Copy link
Contributor

Hi,

quite hard to tell as author bumped from 0.1.3 to 2.0.0. Maybe @phated @hjr3 could give a hint ?

This was me being lazy :)
At the time I wasn't able to put much time into iceddev/pg-connection-string but there were people asking for updates. So I rushed out a major to get people new features when I could have just spent time making sure that the changes didn't break semver.

@brianc has been a great steward of postgres things. I'm happy with whatever the community wants to do with pg-connection-string

@revolunet
Copy link
Contributor Author

i think we can close this one, thank for the update !

@revolunet revolunet closed this Apr 29, 2020
@hjr3 hjr3 mentioned this pull request Jan 17, 2024
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.

6 participants