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

SDAP-35 (completed the configuration change) #6

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@Yongyao
Copy link
Contributor

Yongyao commented Mar 7, 2018

No description provided.

@asfgit

This comment has been minimized.

Copy link

asfgit commented Mar 7, 2018

Can one of the admins verify this patch?

@Yongyao Yongyao changed the title Completed the configuration change SDAP-35 (completed the configuration change) Mar 7, 2018

@lewismc

This comment has been minimized.

Copy link
Contributor

lewismc commented Mar 7, 2018

Hi @Yongyao thank you, in the future can you please (as per the CONTRIBUTING guidelines)

  1. open an issue in JIRA I did this for you SDAP-35
  2. thoroughly describe your issue and tag it with the fix version, components etc.
  3. name your local branch after the JIRA issue (you did this correctly so thank you)
  4. Name the title of the issue after your JIRA issue e.g. SDAP-35 Overhaul MUDROD configuration

Thank you

@lewismc
Copy link
Contributor

lewismc left a comment

@Yongyao you've forced a merge here and it is reverting so much material from the codebase. You need to cherry pick your controbutions and rework a patch. You are introducing too many regressions. Please close this PR and resubmit a clean PR which ONLY includes your overhaul of the configuration.

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.sdap.mudrod.driver.ESDriver;
import org.apache.sdap.mudrod.driver.SparkDriver;
import org.apache.sdap.mudrod.main.MudrodConstants;
import org.apache.sdap.mudrod.weblog.pre.*;

This comment has been minimized.

@lewismc

lewismc Mar 7, 2018

Contributor

Please never use wildcard imports. It is messy and leads to confusion.

@@ -99,11 +99,11 @@ public void preprocess() {

ArrayList<String> inputList = (ArrayList<String>) getFileList(props.getProperty(MudrodConstants.DATA_DIR));

for (String anInputList : inputList) {
timeSuffix = anInputList;
for (int i = 0; i < inputList.size(); i++) {

This comment has been minimized.

@lewismc

lewismc Mar 7, 2018

Contributor

This propose change is actually worse for execution. List comprehension in Java is more performant the way it is. Your proposal slows it down, please revert your change.

@@ -140,8 +140,8 @@ public void preprocess() {
public void logIngest() {
LOG.info("Starting Web log ingest.");
ArrayList<String> inputList = (ArrayList<String>) getFileList(props.getProperty(MudrodConstants.DATA_DIR));
for (String anInputList : inputList) {
timeSuffix = anInputList;
for (int i = 0; i < inputList.size(); i++) {

This comment has been minimized.

@lewismc

lewismc Mar 7, 2018

Contributor

This is a regression and should be reverted.

List<String> customlist = new ArrayList<>();
for (String aList : list) {
customlist.add(this.customAnalyzing(indexName, aList));
for (int i = 0; i < size; i++) {

This comment has been minimized.

@lewismc

lewismc Mar 7, 2018

Contributor

This is a regression please revert.

@@ -223,7 +229,9 @@ public void deleteType(String index, String type) {
String[] indices = client.admin().indices().getIndex(new GetIndexRequest()).actionGet().getIndices();

ArrayList<String> indexList = new ArrayList<>();
for (String indexName : indices) {
int length = indices.length;

This comment has been minimized.

@lewismc

lewismc Mar 7, 2018

Contributor

Please revert this

@@ -260,19 +259,19 @@ public String ssearch(String index, String type, String query, String queryOpera
Gson gson = new Gson();
List<JsonObject> fileList = new ArrayList<>();

for (SResult aLi : li) {
for (int i = 0; i < li.size(); i++) {

This comment has been minimized.

@lewismc

lewismc Mar 7, 2018

Contributor

Revert this

@@ -34,10 +33,10 @@
private static boolean isMultFiles;

private static String[] myHeader;
private static List<List<String>> myMasterList = new ArrayList<>();
private static List<List<String>> myMasterList = new ArrayList<List<String>>();

This comment has been minimized.

@lewismc

lewismc Mar 7, 2018

Contributor

Just use diamond operator don't add type to right hand side of equation


// HashMap used for comparing evaluation classes
public static final Map<String, Integer> map1 = new HashMap<>();
public static final HashMap<String, Integer> map1 = new HashMap<String, Integer>();

This comment has been minimized.

@lewismc

lewismc Mar 7, 2018

Contributor

Return types should not be concrete implementations if possible. Also use diamond operator

@@ -136,7 +135,7 @@ public static void parseFile() {
* @param arr the parsed contents of the original CSV file
*/
public static void calculateVec(String[][] arr) {
List<List<String>> listofLists = new ArrayList<>(); // Holds calculations
List<List<String>> listofLists = new ArrayList<List<String>>(); // Holds calculations

This comment has been minimized.

@lewismc

lewismc Mar 7, 2018

Contributor

Use diamond

@@ -145,6 +144,7 @@ public static void calculateVec(String[][] arr) {
List<String> colList = new ArrayList<String>(); // create vector to store all values inside of a column, which is stored inside 2D vector
for (int col = 0; col < arr[0].length - 1; col++) // Columns go until the next to last column
{
//System.out.println(col + " " + arr[row][col]);

This comment has been minimized.

@lewismc

lewismc Mar 7, 2018

Contributor

Remove line

@Yongyao Yongyao closed this Mar 7, 2018

@Yongyao Yongyao deleted the Yongyao:SDAP-35 branch Mar 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.