sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Fero Szabo via Review Board <nore...@reviews.apache.org>
Subject Re: Review Request 65530: MySQL thirdparty tests hang if there's no USER environment variable
Date Mon, 12 Feb 2018 16:53:28 GMT


> On Feb. 12, 2018, 4:50 p.m., Fero Szabo wrote:
> > Hi Dani,
> > 
> > Great catch!
> > 
> > I like in your solution that it's greatly simplified compared to the original code.
However, I believe that the process that executes the whoami command is never destroyed and
hangs around in the background, according to the Process class' documentation (subprocesses
with no reference to them are not destroyed):
> > 
> > https://docs.oracle.com/javase/7/docs/api/java/lang/Process.html
> > 
> > Probably this is why the original while loop existed (?) I'm really just guessing.
> > 
> > Anyway, I find it strange to use whoami to get the username here, as this username
is later on usedd by DirectMySQLManager. So this username is probably for the database, which
is usually different than what whoami returns (at least on my system, it is). Better to throw
an exception if it's not set?
> > 
> > Cheers,
> > Fero

I have to add that I tested your diff through DirectMySQLExportTest, so it might make sense
to return the output of whoami elsewhere...


- Fero


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65530/#review197280
-----------------------------------------------------------


On Feb. 6, 2018, 12:15 p.m., daniel voros wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65530/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2018, 12:15 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3283
>     https://issues.apache.org/jira/browse/SQOOP-3283
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> `org.apache.sqoop.manager.mysql.MySQLTestUtils#getCurrentUser()` executes `whoami` in
a subprocess if there's no USER environment variable (happened to me while running tests from
Docker). However, it waits for the Process variable to become null, that never happens:
> 
> ```
> // wait for whoami to exit.
> while (p != null) {
>   try {
>     int ret = p.waitFor();
>     if (0 != ret) {
>       LOG.error("whoami exited with error status " + ret);
>       // suppress original return value from this method.
>       return null;
>     }
>   } catch (InterruptedException ie) {
>     continue; // loop around.
>   }
> }
> ```
> 
> We could get rid of the while loop since `Process#waitFor()` blocks while it completes.
> 
> Note, that it's easy to workaround the issue by setting the USER environment variable
when running the tests.
> 
> 
> Diffs
> -----
> 
>   src/test/org/apache/sqoop/manager/mysql/MySQLTestUtils.java 25dbe9d 
> 
> 
> Diff: https://reviews.apache.org/r/65530/diff/1/
> 
> 
> Testing
> -------
> 
> Run `org.apache.sqoop.manager.mysql.MySQLCompatTest`. Failed with timout without the
patch. All 46 test cases pass in ~45 seconds with the patch in place.
> 
> 
> Thanks,
> 
> daniel voros
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message