search

sipcapture / heplify-server

HEP Capture Server for HOMER
https://sipcapture.org
GNU Affero General Public License v3.0
184 stars 85 forks source link

PgSQL dsn improvements #439

Closed volga629-1 closed 3 years ago

volga629-1 commented 4 years ago

Ticket reference: #437

CLAassistant commented 4 years ago

CLA assistant check
All committers have signed the CLA.

nathanleyton commented 3 years ago

Hi I am happy to see find a PR for this as we really need it. Do we have a lead time on when this might be merged? Not having sslmode means we cannot use homer. Thanks

negbie commented 3 years ago

DBPort is a breaking change so can't merge.

volga629-1 commented 3 years ago

DBPort is a breaking change so can't merge.

Can you please point what exactly is not right ?

negbie commented 3 years ago

In the old config you use DBAddr as "localhost:1234" with your PR you use DBAddr as "localhost" and DBPort "1234"

volga629-1 commented 3 years ago

Yes, DBPort is part of pull request, I added corresponding options to config.go. My pull request I added to spec file and everything works as expected. I am not sure what actually is breaks. It just separate option for host and port to make dsn more flexible.

negbie commented 3 years ago

There are alot of users who have config files with DBAddr set to for example "localhost:1234".

lmangani commented 3 years ago

Fully agree with @negbie there's no reason to break backwards compatibility for such a minimal change with no particular advantage. @volga629-1 could you merge those back or make the second an optional parameter to retain compatibility?

ZionDials commented 3 years ago

Hello, I created a pull request that allows SSL Mode for the DB Connection without breaking backward compatibility. #449

negbie commented 3 years ago

Guys, you should read more carefully, this PR should not be approved but ZionDials PR https://github.com/sipcapture/heplify-server/pull/449

adubovikov commented 3 years ago

@negbie oki! thanks for pointing :)

negbie commented 3 years ago

https://github.com/sipcapture/heplify-server/pull/449 was merged so closing this. Thanks @volga629-1 for bringing this up.