Add postgresql support #372
Conversation
| @@ -84,23 +116,28 @@ public Properties getDatasourceProperties() { | |||
| } | |||
| } | |||
|
|
|||
| // TODO: Are defaults adding any value here? Should we just eliminate them? | |||
There was a problem hiding this comment.
I don't think these add much value, and they will need to be customized in pretty much every scenario anyways (even local testing ones). Also, it would be signing Boost up for maintaining a list of defaults for all the different databases which I don't think you guys will want to be on the hook for.
I vote we eliminate this part
There was a problem hiding this comment.
Hmm... I think I agree. Even if they were a bit more often likely to be used (maybe in Docker scenarios) I'm hesitant to extend Boost in this direction. Plus it's not even a straightforward default.. there's non-trivial logic around when to use it (if none of the other props are set).... let's hold off on this for now. I wonder if we need to tweak any existing ITs then.
There was a problem hiding this comment.
Agree. I think we should only default values for the Derby embedded case. Being able to configure the database name for derby was more of an add on any way. We should allow this to work without any user input.
For non-Derby drivers, if no properties are provided, we can throw a warning and create empty variables for url / user / password that can be set at runtime. (I think those are the only required ones)
awisniew90
force-pushed
the
postgresql
branch
from
d74f6f3
to
4eab5bb
Compare
|
Created #377 to have the test-jdbc IT use MicroShed Testing to start containerized databases rather than using the docker-maven-plugin. As a part of this, we should add a PostgreSQL test. |
No description provided.