Skip to content
This repository was archived by the owner on Jan 8, 2019. It is now read-only.

[Bug] Fix hive-site.xml URI generation#817

Merged
pu239ppy merged 4 commits intobloomberg:masterfrom
vt0r:bugfix/hive-site_metastore_uris
Mar 27, 2017
Merged

[Bug] Fix hive-site.xml URI generation#817
pu239ppy merged 4 commits intobloomberg:masterfrom
vt0r:bugfix/hive-site_metastore_uris

Conversation

@vt0r
Copy link
Member

@vt0r vt0r commented Mar 23, 2017

As usual when dealing with older files, this change contains the actual changes in separate commits from the cleanups.

  • First commit makes the change to fix the metastore URI generation logic, prepending the protocol to all hosts in the string as opposed to just the first one in the list.
  • Second commit cleans the file to comply with all Rubocop/Foodcritic rules other than line length.
  • Third commit came after I realized a similar issue was present in the MySQL loadbalance URI generation logic, which would only append the port number to the last host. Luckily, this issue does not have any real impact, as we're using the default port of 3306. However, should we decide to move to a nonstandard port at any point in the future, we will now be able to replace the ':3306' string with an attribute, and the string will be created properly.

vt0r added 3 commits March 23, 2017 12:23
- Was previously only prepending thrift:// to a single host.
...per [the docs](https://dev.mysql.com/doc/connector-j/5.1/en/connector-j-usagenotes-j2ee-concepts-managing-load-balanced-connections.html). This was not causing any issues luckily, since 3306 is the default and used when no port is specified, but if we ever decide to use a non-standard port, we will want to be able to replace this string with an attribute and append it to each host accordingly.
@vt0r vt0r added the Tested label Mar 23, 2017
@pu239ppy
Copy link
Contributor

Looks good

@pu239ppy pu239ppy closed this Mar 27, 2017
@pu239ppy pu239ppy reopened this Mar 27, 2017
@pu239ppy
Copy link
Contributor

Looks good

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants