exadel-inc / CompreFace

Leading free and open-source face recognition system
https://exadel.com/accelerator-showcase/compreface/
Apache License 2.0
5.22k stars 719 forks source link

Compreface-API rewrites my postgresql jdbc url #1213

Open DrissiReda opened 9 months ago

DrissiReda commented 9 months ago

Describe the bug

I supplied a custom jdbc url to my already running postgresql host. However the host name includes postgresql which gets rewritten into psql for some reason.

The compreface-api container image from version 0.6.0 until the latest has this issue, 0.5.1 works as intended.

Is that normal? Is there some code that replaces every string postgresql in the configuration by psql?

To Reproduce

Steps to reproduce the behavior:

  1. Include postgresql in the host part of the environment variable POSTGRES_URL.
  2. Profit

Expected behavior

My url is supposed to be used as-is, not overwritten by something I cannot control

compreface-admin does not have this issue, regardless of the version.

pospielov commented 8 months ago

We don't do any replacements on our side. We use Java Spring for data connection and pass this variable straight to Spring: https://github.com/exadel-inc/CompreFace/blob/master/java/admin/src/main/resources/application.yml#L39C30-L39C40 I think it may be a Spring, or even JDBC bug.

DrissiReda commented 8 months ago

While I'm not familiar with spring. I'm using a lot of projects that use JDBC and this is the first time I've ever had this issue with my setup...

newbenji commented 4 months ago

Actually i have same problem this is set as env for both api and for admin name: POSTGRES_URL value: >- jdbc:postgresql://postgresql-rw.postgresql.svc.cluster.local:5432/compreface

and this is logerror from api java.sql.SQLException: Connection Error: address 'pgsql-rw.pgsql.svc.cluster.local' is unresolved

newbenji commented 4 months ago

Can tell, that when i renamed the service and the namespace, ( was also postgresql ) everything worked as it should for api also.

newbenji commented 4 months ago

Actually i have same problem this is set as env for both api and for admin name: POSTGRES_URL value: >- jdbc:postgresql://postgresql-rw.postgresql.svc.cluster.local:5432/compreface

and this is logerror from api java.sql.SQLException: Connection Error: address 'pgsql-rw.pgsql.svc.cluster.local' is unresolved

you can see all postgresql was replaced with pgsql

DrissiReda commented 4 months ago

@newbenji I have only managed to circumvent it by adding another Kubernetes service, you know, just like an alternate dns, it works.

newbenji commented 4 months ago

@newbenji I have only managed to circumvent it by adding another Kubernetes service, you know, just like an alternate dns, it works.

Did the same, but sadly also needed new namespace. was postgresql :-D But was new postgresqlsetup so was not so big a deal

DrissiReda commented 4 months ago

@newbenji I have only managed to circumvent it by adding another Kubernetes service, you know, just like an alternate dns, it works.

Did the same, but sadly also needed new namespace. was postgresql :-D But was new postgresqlsetup so was not so big a deal

as you've understood, it really overwrites any string postgresql in the provided url, probably to correct the protocol (e.g replace postgresql://.... by psql://....)

newbenji commented 4 months ago

@newbenji I have only managed to circumvent it by adding another Kubernetes service, you know, just like an alternate dns, it works.

Did the same, but sadly also needed new namespace. was postgresql :-D But was new postgresqlsetup so was not so big a deal

as you've understood, it really overwrites any string postgresql in the provided url, probably to correct the protocol (e.g replace postgresql://.... by psql://....)

actually its not looking at :// it just replace all postgresql with psql

newbenji commented 4 months ago

But weird it dosnt happen for admin

treaded commented 4 months ago

Hi,

I have encountered the same issue as described in this thread. After investigating the cause, I believe the following code is responsible:

NotificationDbConfig.java#L15 replace()method replaces every matched string.

It seems that this part of the code is causing the problem. Could you please take a look into this?

Thank you!

DrissiReda commented 4 months ago

@pospielov could you see if it can be removed or modified? Why is it necessary for this commit: https://github.com/exadel-inc/CompreFace/commit/f9f0f33708c9381591d0bcec52d1ebddb2b5a511 ?