bbeaudreault commented on pull request #4180: URL: https://github.com/apache/hbase/pull/4180#issuecomment-1078974977
Trying to be thorough here -- I've been doing some testing of the new Configuration.addDeprecation. If we want to rely on the functionality it provides,unfortunately I think the only truly safe place to put this is in HBaseConfiguration: - We basically have to call addDeprecation either at or before the deprecated `HBASE_CLIENT_PAUSE_FOR_CQTBE` constant is accessed. So at a minimum it should be in HConstants. - But actually, one could have configured `hbase.client.pause.cqtbe` in their hbase-site.xml, in which case HConstants may not get initialized in time. So if we want to be absolutely sure that all potential configuration points are handled, we need to call it before hbase-site.xml is loaded. - Even still, I suppose it's possible for someone to launch their application with `-Dhbase.client.pause.cqtbe=` which might set it before hbase-site.xml is loaded, not sure. It's too bad -- it would be nice to clean up the `config.get(newName, config.get(oldName, default))`. But given the above, I'm going to add that back in for now. In terms of whether or where we still call addDeprecation, neither HConstants nor HBaseConfiguration are very satisfying to me -- they both exist in hbase-common, but the new config exists in hbase-client. I guess this is the problem with HConstants :). I may leave it where I have it, in ConnectionConfiguration, as sort of an additional self-documenting fact. Alternatively I could move it but would have to duplicate the `hbase.client.pause.server.overloaded` raw string. In the interest of having something for someone to review, I'm going to add the fallback handling and leave the deprecation where it is. If anyone feels strongly for a different solution, I will be happy to change it. Also, I've decided to remove the `LP(Config)` change from this PR, since that debate is still ongoing. It doesn't so much matter here, as I can still just expose the public constant in the LP(Private) class. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
