Skip to content

KNOX-2156 - CM discovery - KUDUUI discovery#228

Merged
pzampino merged 1 commit intoapache:masterfrom
pzampino:master
Dec 20, 2019
Merged

KNOX-2156 - CM discovery - KUDUUI discovery#228
pzampino merged 1 commit intoapache:masterfrom
pzampino:master

Conversation

@pzampino
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Added support for discovering URLs for the Apache Kudu UI.

How was this patch tested?

Added tests to ClouderaManagerServiceDiscoveryTest, and manually tested.

String scheme = sslEnabled ? "https" : "http";

String port = getRoleConfigValue(roleConfig, "webserver_port");
return createServiceModel(String.format(Locale.getDefault(), "%s://%s:%s/", scheme, hostname, port));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this is in others and I missed it. I think Locale.ROOT is probably better here. I don't think we need to use localization for any of the parameters here. We should just plop in as is.

Note 99% sure we won't easily run into this since there are only a few locales where there is a differences.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Local.getDefault() is used in the others. We can revisit that for all of them.

@pzampino pzampino merged commit 6aab5de into apache:master Dec 20, 2019
stoty pushed a commit to stoty/knox that referenced this pull request May 14, 2024
Change-Id: I0b89fd3a3966c8608d6c76c1669474169fa6b5b0
stoty pushed a commit to stoty/knox that referenced this pull request May 14, 2024
* changes:
  KNOX-2156 - CM discovery - KUDUUI discovery (apache#228)
  KNOX-2152 - Disable Ambari cluster configuration monitoring by default (apache#225)
  KNOX-2151 - HIVE_ON_TEZ HS2 Discovery doesn't work (apache#224)
  KNOX-1970 - CM discovery - Add Impala HS2 to auto discovery (apache#223)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants