Message-ID: From: "jrobe (@jrobe)" To: "pgjdbc/pgjdbc" Date: Thu, 29 Jun 2023 17:34:48 +0000 Subject: Re: [pgjdbc/pgjdbc] PR #2923: un-deprecate SSL_FACTORY_ARG In-Reply-To: References: List-Id: X-GitHub-Author-Login: jrobe X-GitHub-Comment-Id: 1613576715 X-GitHub-Comment-Type: issue_comment X-GitHub-Issue: 2923 X-GitHub-Repo: pgjdbc/pgjdbc X-GitHub-Type: comment X-GitHub-Url: https://github.com/pgjdbc/pgjdbc/pull/2923#issuecomment-1613576715 Content-Type: text/plain; charset=utf-8 > I do not think so. SingleCertValidatingFactory can receive arguments with Properties just fine, and it does not require sslfactoryarg. It does require that arg today; and no publicly settable property exists to configure it besides sslfactoryarg. > There should be a better way to pass custom properties than sslfactoryarg. I mean, this may be true; but why is sslfactoryarg so inherently bad? Since we can define our own SSLFactory to use, we should have a property that we can use to give context to our own SSLFactory class. We'd just be trading one property for another, the other one being whatever custom stuff is defined in a later PR. > I have no issues with merging this PR, however, I would raise a new one that removes the usages of sslfactoryarg from pgjdbc code and deprecates the property again. Why discuss un-deprecating it? That is exactly the reason I suggest we stop discussing "un-deprecation", and we should rather focus on the way how clients should pass extra properties. > I assume "un-deprecating and deprecating it again" would be pure noise, especially taking into account that we won't release pgjdbc in-between. This does indeed assume there's a simple answer to customer user properties; however I don't think that'll be as straightforward as you're making it out to seem; since BaseDataSource would need to have a way to hold all of these custom properties and send them around to the Driver, the SSLSocketFactory, and other classes. Can we please merge this PR and only deprecate it if there is indeed a workable other solution that is released later? And removing usages of sslfactoryarg from pgjdbc code is not a valid reason for deprecation; since sslFactory is allowed to be any custom class and users still need a way to send in arguments to their custom class (and removing that existing constructor is a non-backwards compatible change which is a huge uplift for seemingly no reason). Deprecation should only happen if there's an alternate way to configure any custom SSLFactory (not just the SingleCertFactory).