-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDDS-1093. Configuration tab in OM/SCM ui is not displaying the correct values #527
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
Conversation
@bharatviswa504 @avijayanhwx @arp7 Please review when you find time |
hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/conf/TestOzoneConfiguration.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/conf/TestOzoneConfiguration.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/conf/TestOzoneConfiguration.java
Outdated
Show resolved
Hide resolved
--
Sent from my Android phone with GMX Mail. Please excuse my brevity.On 2019-02-27, 9:55 p.m. Siddharth <notifications@github.com> wrote:
@swagle commented on this pull request.
In hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/conf/TestOzoneConfiguration.java:
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+
+public class TestOzoneConfiguration {
+
+ private Configuration conf;
+ final static String CONFIG = new File("./test-config-TestConfiguration.xml").getAbsolutePath();
+ final static String CONFIG_CORE = new File("./core-site.xml").getAbsolutePath();
+
+ private BufferedWriter out;
In general not too excited about sharing streams as a coding best practice, others reviewers can chime in but better to localize this vs deal with side effect code in tearDown.
—You are receiving this because you are subscribed to this thread.Reply to this email directly, view it on GitHub, or mute the thread.
{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/apache/hadoop","title":"apache/hadoop","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/apache/hadoop"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swagle commented on #527"}],"action":{"name":"View Pull Request","url":"#527 (review)"}}}
[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "#527 (review)",
"url": "#527 (review)",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]
|
💔 -1 overall
This message was automatically generated. |
@@ -161,4 +163,31 @@ public static void activate() { | |||
Configuration.addDefaultResource("ozone-default.xml"); | |||
Configuration.addDefaultResource("ozone-site.xml"); | |||
} | |||
|
|||
/** | |||
* The super class method getAllPropertiesByTag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need an other (HADOOP) jira to fix it on the hadoop 3.3 line, too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes @elek
I am just wondering if it's the same issue as https://issues.apache.org/jira/browse/HDDS-611 |
Thanks @elek ! Marked it a duplicate. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall patch LGTM.
I tested this patch on docker cluster, now I am able to see updated values for the properties.
Can you also open a HADOOP JIRA to fix this in hadoop-common?
One minor comment about imports, once that is addressed I am +1 with the change.
hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/conf/TestOzoneConfiguration.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/conf/TestOzoneConfiguration.java
Outdated
Show resolved
Hide resolved
💔 -1 overall
This message was automatically generated. |
@vivekratnavel |
💔 -1 overall
This message was automatically generated. |
Thank You @vivekratnavel for the fix. |
💔 -1 overall
This message was automatically generated. |
In samza-sql module, currently few test classes(`TestSamzaSqlRelMessageSerde` and `TestSamzaSqlRelRecordSerde`) are dependent upon `javafx.util.Pair` class(coming from `javafx` module). `javafx.util.Pair` is not supported by default in all JDK builds(example; open-jdk java-8 doesn't support `javafx` module) and it belongs to `javafx` package which is primarily used for developing GUI applications. This dependency is removed and replaced with `Pair` class from `apache-commons`. Author: Shanthoosh Venkataraman <santhoshvenkat1988@gmail.com> Reviewers: Jagadish V <vjagadish@gmail.com> Closes apache#527 from shanthoosh/SAMZA-1720
Configuration tab in OM/SCM ui is not displaying the correct/configured values, rather it is displaying the default values.