> This looks like really cool functionality.
Thanks! Yes it's a pretty straightforward idea and was kind of
surprising that it didn't already exist.
I submitted a similar patch to the MariaDB JDBC driver project as well
though the final implementation they merged[1] in is significantly
less extensible than this (basically only the string format is
allowed). Although we only use the string format of entering a
certificate in JackDB I can see folks using all the other ones as well
(ex: the "env:" method jives with modern PaaS coding standards) and
like this version much better.
> My main concern with the code as written is that testing it requires manual steps, including booting a virtual
machineand running a separate build.
>
> In my experience, any steps beyond the simple "run all the tests" step will never be done. Human nature.
You're right and it definitely should be included into the main build
and automated. That's one of the reasons I split up the new code into
a patch for the factory itself and one for the testing/VM. The latter
is more of a template for a proper test harness and really should be
integrated with the rest of the driver build/test process.
> Would it be possible to convert the test case to run as a part of the normal test suite?
Of course! I'll be honest, I wasn't quite sure how to integrate it
with the existing ant build, hence the separate VM and Maven project
for testing (e.g. not sure if there are separate build/test server for
the JDBC driver).
Still though I think having a built-in VM in the project for folks to
spin up for testing would be a good idea. It makes it *a lot* easier
to test out code changes. Plus it'd be possible to have multiple VMs
with different versions of PG installed (to properly test against all
the combinations of PG versions/drivers). If there's a CI server in
place the test VM should match up with that.
> There are also a couple of scary places where exceptions are swallowed, e.g. SingleCertTrustManager#<init>
The only swallowed exception is in the constructor for
SingleCertTrustManager and that's only when calling the "load" method
of the KeyStore to initialize it (with a bogus null param). It looks
like a no-op but it's required as otherwise the KeyStore won't be
initialized and you can't add certificates to it afterwards.
Thanks,
Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | http://www.jackdb.com/ | @jackdb
[1]: https://kb.askmonty.org/en/mariadb-java-client-113-changelog
