Details

    • Type: Test Test
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.4.0
    • Fix Version/s: 0.6.0
    • Component/s: None
    • Labels:
      None

      Description

      Wrote a test that checks whether the distcp command and its suboptions (using a source file etc.) work without error and confirms expected output is correct.

        Activity

        Hide
        Roman Shaposhnik added a comment -

        +1 and committed!

        Show
        Roman Shaposhnik added a comment - +1 and committed!
        Hide
        Sujay Rau added a comment -

        New patch for distcp test

        Show
        Sujay Rau added a comment - New patch for distcp test
        Hide
        Sujay Rau added a comment -

        fixing sh.getRet and package issue

        Show
        Sujay Rau added a comment - fixing sh.getRet and package issue
        Hide
        Roman Shaposhnik added a comment -

        Same comment as before – unless I'm missing something this doesn't seem to have been tested:

          @BeforeClass
          public static void setUp() {
            JarContent.unpackJarContainer(TestDistCpIntra.class, "." , null);
            // get namenode hostname from core-site.xml
            // to manually specify hostname, comment out this section and set namenode to "hdfs://...", and comment out HADOOP_HOME/CONF_DIR Block above
            Configuration conf = new Configuration();
            namenode = conf.get("fs.defaultFS");
            if (sh.getRet() != 0) {
        

        Also, while you're at it, could you please fix things like:

        sh.exec("hadoop distcp " + namenode + "/user/$USERNAME/$testDistcpInputs" + "4/$dcpfile" + "4.txt " + namenode + "/user/$USERNAME/$testDistcpInputs" + "5/$dcpfile" + "5.txt " + namenode + "/user/$USERNAME/$testDistcpOutputs");
        

        to not use '+' and substitute within a Groovy string with a dollar sign instead. It is way easier to review that way.

        Show
        Roman Shaposhnik added a comment - Same comment as before – unless I'm missing something this doesn't seem to have been tested: @BeforeClass public static void setUp() { JarContent.unpackJarContainer(TestDistCpIntra.class, "." , null); // get namenode hostname from core-site.xml // to manually specify hostname, comment out this section and set namenode to "hdfs://...", and comment out HADOOP_HOME/CONF_DIR Block above Configuration conf = new Configuration(); namenode = conf.get("fs.defaultFS"); if (sh.getRet() != 0) { Also, while you're at it, could you please fix things like: sh.exec("hadoop distcp " + namenode + "/user/$USERNAME/$testDistcpInputs" + "4/$dcpfile" + "4.txt " + namenode + "/user/$USERNAME/$testDistcpInputs" + "5/$dcpfile" + "5.txt " + namenode + "/user/$USERNAME/$testDistcpOutputs"); to not use '+' and substitute within a Groovy string with a dollar sign instead. It is way easier to review that way.
        Hide
        Sujay Rau added a comment -

        Fixed shell issue

        Show
        Sujay Rau added a comment - Fixed shell issue
        Hide
        Roman Shaposhnik added a comment -

        Just supply a single argument of the name of the shell and you should be fine. Oh and you're right it seems that BIGTOP-625 also suffers from the same issue. Could you, please, update the patch there as well?

        Show
        Roman Shaposhnik added a comment - Just supply a single argument of the name of the shell and you should be fine. Oh and you're right it seems that BIGTOP-625 also suffers from the same issue. Could you, please, update the patch there as well?
        Hide
        Sujay Rau added a comment -

        No you are right. Is there a username I should specify if it is just a regular account - or do I leave it blank? (I will need to quickly change this for one or two of the other patches as well)

        Show
        Sujay Rau added a comment - No you are right. Is there a username I should specify if it is just a regular account - or do I leave it blank? (I will need to quickly change this for one or two of the other patches as well)
        Hide
        Roman Shaposhnik added a comment -

        The patch looks good. One last question – is it really necessary for the shell to be instantiated under the root account? While reviewing the patch I haven't found anything that would require super-user privileges.

        Show
        Roman Shaposhnik added a comment - The patch looks good. One last question – is it really necessary for the shell to be instantiated under the root account? While reviewing the patch I haven't found anything that would require super-user privileges.
        Hide
        Sujay Rau added a comment -

        Updated patch by your suggestions.

        Show
        Sujay Rau added a comment - Updated patch by your suggestions.
        Hide
        Sujay Rau added a comment -

        Thanks for the feedback. Yea I just recently learned how the Configuration class works but forgot to go back and fix it. I have I believe 5 tests in total that I'm looking to submit but I'll make all these changes to all of them and post better patches later in the day hopefully.

        Show
        Sujay Rau added a comment - Thanks for the feedback. Yea I just recently learned how the Configuration class works but forgot to go back and fix it. I have I believe 5 tests in total that I'm looking to submit but I'll make all these changes to all of them and post better patches later in the day hopefully.
        Hide
        Roman Shaposhnik added a comment -

        A couple more comments:

        1. "private static final String USERNAME = sh.exec("whoami").getOut().get(0);" – nor reason to do this since System.getProperty( "user.name" ) would do just fine
        2. "+ execute("cat $HADOOP_CONF_DIR/core-site.xml");" – I would suggest parsing it with either Groovy XML slurper or better yet native Hadoop Configuration class
        3. "execute("hadoop fs -test -e testDistcpInputs");" – I would suggest randomizing the naming of test directories/files
        Show
        Roman Shaposhnik added a comment - A couple more comments: "private static final String USERNAME = sh.exec("whoami").getOut().get(0);" – nor reason to do this since System.getProperty( "user.name" ) would do just fine "+ execute("cat $HADOOP_CONF_DIR/core-site.xml");" – I would suggest parsing it with either Groovy XML slurper or better yet native Hadoop Configuration class "execute("hadoop fs -test -e testDistcpInputs");" – I would suggest randomizing the naming of test directories/files
        Hide
        Sujay Rau added a comment -

        Ok, I will remove them and re-submit.

        Show
        Sujay Rau added a comment - Ok, I will remove them and re-submit.
        Hide
        Roman Shaposhnik added a comment -

        I don't think there's any reason to introduce helper methods for debugging since the built-in shell class does full logging (provided that loglevel for it is set to trace)

        Show
        Roman Shaposhnik added a comment - I don't think there's any reason to introduce helper methods for debugging since the built-in shell class does full logging (provided that loglevel for it is set to trace)
        Hide
        Sujay Rau added a comment -

        Added patch for my test.

        Show
        Sujay Rau added a comment - Added patch for my test.

          People

          • Assignee:
            Sujay Rau
            Reporter:
            Sujay Rau
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development