> On April 25, 2017, 5:43 p.m., Kirk Lund wrote:
> > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
> > Lines 238 (patched)
> > <https://reviews.apache.org/r/58589/diff/3/?file=1698725#file1698725line239>
> >
> >     Insert one line above the throw to restore the interrupt bit on the 
> > current thread:
> >     ```java
> >     Thread.currentThread().interrupt();
> >     ```
> 
> Jinmei Liao wrote:
>     You mean like this?
>     .....
>         } catch (final InterruptedException e) {
>           Thread.currentThread().interrupt();
>           throw new IllegalStateException(e.getMessage(), e);
>         }
>         
>         What would this do?

Yep, that's correct. That just resets the interrupt bit. Anytime you catch 
InterruptedException you're supposed to reset the interrupt bit like that. I 
can probably find some more info online. If you own the Thread and the Thread 
is going to completely terminate (exit its run() method) then you don't need to 
reset the bit.

If the bit is reset, then the next time that Thread invokes a call that checks 
the interrupt bit, the Thread will once again throw InterruptedException. This 
should bring up more examples in Geode:

git grep "Thread.currentThread().interrupt()"


- Kirk


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


On April 26, 2017, 7:48 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58589/
> -----------------------------------------------------------
> 
> (Updated April 26, 2017, 7:48 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-1597: use Spring shell's parser and delete our own parsing code
> 
> * deleted usage of jopsimple and our own parser code
> * reworked help/hint
> 
> The biggest change is in GfshParser, instead of directly implement 
> SpringShell's Parser, now it's just a simple wrapper around SpringShell's 
> SimpleParser so that we can use spring's parsing code. The challenge is that 
> SimpleParser expects the user inupt to be in the format of "command option1 
> value1 option2 value2", while gfsh expects the format to be like "command 
> option1=value1 option2=value2", so most of the code in GfshParser is to turn 
> the input into the SimpleParser input format and then feed it into 
> SimpleParser to get the validation and autocompletion we needed.
> 
> So far the difference I noticed with this new implementation are:
> 1) option validation is what we gain from directly using SpringShell.
> 2) SpringShell doesn't allow you to specify an option twice, so for 
> multivalued parameters, our old parser can accept command like "change 
> loglevel --member=member1 --member=member2", but now the parser will give you 
> an error, and you should only do "change loglevel --member=member1,member2". 
> The new parser did something speical for --J option though, so for --J, you 
> can specify it multiple times. 
> 3) For value separator, springShell default's to ",", you can only overwrite 
> it with option context "splittingRegex", we do not honor the 
> @CliMetaData(valueSepartor=) anymore. All of our commands uses "," for 
> separators, so this won't break our commands, but if there is any command out 
> there that would define a different @CliMetaData(valueSepartor=) other than 
> ",", SpringShell would not know how to parse it.
> 4) To specify empty string as parameter value, before you will need to to 
> (put region=A key="''" value="''"), spring shell would think the value you 
> are trying to pass in are two single quotes instead of empty strings, now, 
> you should only do (put region=A key="" value="")
> 
> 
> Diffs
> -----
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
>  44004454ef1646bfeca8a7af3cffb109fd83e7d7 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
>  a1d03e45dd4d6559bd9a0869c7dd95908d1858ca 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHelpCommands.java
>  1d1b28e568a1e175690fea5cde1a45a318b6db5d 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java
>  b37feabe6ed61c081e9653c94128f092ad189d10 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java
>  c346eaf77087102f51952a567ecd7ec036593a13 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
>  bcf1b415e9ee85822c470ae6888920f0a90159fd 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/CommandManagerJUnitTest.java
>  503ffb2929758d617e8539e6aefb3f7b545de9b6 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserIntegrationTest.java
>  f3e3bd8754e18d7a75faf4b75e3ba75b778fc9d7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserJUnitTest.java
>  44e99f461b9efc5a439e629751011a6d0edd9213 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java
>  165f66482758dadb75ae95d23443973e9de05891 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/help/HelpBlockUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/help/HelperUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategyJUnitTest.java
>  54c7cf7074435b48232b61916627eed69a201f09 
> 
> 
> Diff: https://reviews.apache.org/r/58589/diff/4/
> 
> 
> Testing
> -------
> 
> precheckin successful with the latest changes.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Reply via email to