Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename Hive connector "hive-hadoop2 to "hive" #8166

Merged
merged 1 commit into from Jun 2, 2021

Conversation

electrum
Copy link
Member

@electrum electrum commented Jun 1, 2021

This simplification is long overdue. The original name is kept for compatibility with existing configurations, but a warning is logged when it is used. The plugin directory is also renamed inside the server tarball. We should name the Maven artifact as well, but I'll leave that for a follow up, as it's not obvious what to call it.

@cla-bot cla-bot bot added the cla-signed label Jun 1, 2021
@electrum
Copy link
Member Author

electrum commented Jun 1, 2021

cc @mosabua

@mosabua
Copy link
Member

mosabua commented Jun 1, 2021

Wow ...

@@ -174,13 +174,13 @@ Configuration
-------------

Create ``etc/catalog/hive.properties`` with the following contents
to mount the ``hive-hadoop2`` connector as the ``hive`` catalog,
to mount the ``hive`` connector as the ``hive`` catalog,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these really the only instances of the connector name in all the docs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@findepi
Copy link
Member

findepi commented Jun 1, 2021

from @electrum 's #5625 (review)

At a minimum, we also need to rename the plugin/hive-hadoop2 directory in the tarball. This will be a breaking change for people that do things like add libraries

are we ok with the breaking change?
cc @trinodb/maintainers

@electrum
Copy link
Member Author

electrum commented Jun 1, 2021

@findepi sorry, I completely forgot about your PR, though I see you closed it. I'm glad you remembered my comment about this breaking people that install libraries. We should call this out, but I'm not concerned, since that's an "unsupported" activity and needs to be handled carefully anyway.

@electrum
Copy link
Member Author

electrum commented Jun 1, 2021

We could go a different way and add a getLegacyNames() method to ConnectorFactory, similar to @LegacyConfig in Airlift config. This might be useful for other connectors like MemSQL, for which this approach wouldn't work. But we can always do that later and migrate the Hive connector to use the new mechanism.

@electrum electrum merged commit 022a266 into trinodb:master Jun 2, 2021
@electrum electrum deleted the hive-connector branch June 2, 2021 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants